[llvm][ADT] Overload output stream operator `<<` for `StringMapEntry` and `StringMap`.

[wyt]

Summary

Printing support enables the production of more useful error messages in unit testing e.g. when using matchers such as UnorderedElementsAre() to inspect the contents of a StringMap.

[wyt]

Check if << operator is overloaded for value type, return a default if << is unimplemented.

[gribozavr2]

Please clarify in the CL description that the more useful error messages will appear in unit test failures, for example when UnorderedElementsAre() is used with StringMap.

llvm/include/llvm/ADT/StringMap.h

502 ↗

(On Diff #455963)

llvm/include/llvm/ADT/StringMapEntry.h

150 ↗

(On Diff #455963)

WDYT about "CanOutputToOstream"?

150 ↗

(On Diff #455963)

Similar to hiding implementation details in DenseMap.h and in other headers, I'd suggest to put this type trait inside namespace detail {}.

163 ↗

(On Diff #455963)

166 ↗

(On Diff #455963)

I wonder if we should put these operators into llvm/Testing/ADT, since these operators are only supposed to be used from tests. For production code, LLVM strongly prefers llvm::raw_ostream, a much more light-weight type than std::ostream.

In fact, this is the first operator<<(std::ostream&, <anything>) overload in llvm/ADT. Printable types (like APInt) are printable into raw_ostream.

Another option is to provide operator<<(llvm::raw_ostream&) in llvm/ADT, and wrap them in operator<<(std::ostream&) in llvm/Testing/ADT. The llvm/ADT version won't have the SFINAE magic, but the llvm/Testing/ADT one will.

WDYT?

[wyt]

Address comments.

[wyt]

llvm/include/llvm/ADT/StringMapEntry.h

166 ↗

(On Diff #455963)

Moved the printing into Testing/ADT.

I also attempted the second approach but it wasn't very successful. Using <<(llvm::raw_ostream) from <<(std::ostream) meant that <<(llvm::raw_ostream) needed to be implemented for any structure we wanted to print. However for use cases related to google test, it is more likely that users implement the <<(std::ostream) operator and not the <<(llvm::raw_ostream).

[gribozavr2]

llvm/include/llvm/ADT/StringMapEntry.h

166 ↗

(On Diff #455963)

Using <<(llvm::raw_ostream) from <<(std::ostream) meant that <<(llvm::raw_ostream) needed to be implemented

What I meant is that when implementing <<(std::ostream) you can call <<(llvm::raw_ostream) printing into a string, and then print the string into the std::ostream.

But that's not necessary, what you have now also works.

[wyt]

Rename build target.

[ymandel]

llvm/include/llvm/Testing/ADT/StringMap.h

27

Is there a standard representation for empty maps and, if so, is this it?

[wyt]

llvm/include/llvm/Testing/ADT/StringMap.h

27

I'm not aware of one, imo { } seemed pretty representative of maps.
Do you have a preference?

[wyt]

Minor fixes:

• Update sfinea to check on ostream& and use decltype to get type of the input to be streamed
• Fix build target dependency

[dblaikie]

llvm/include/llvm/Testing/ADT/StringMap.h

27

do we need the special case for empty, or can it flow out of the general case naturally?

[wyt]

llvm/include/llvm/Testing/ADT/StringMap.h

27

The special case keeps the braces on the same line for readability, otherwise they would be on separate lines.

llvm/include/llvm/Testing/ADT/StringMap.h

+//===- llvm/Testing/ADT/StringMap.h ---------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TESTING_ADT_STRINGMAP_H_
+#define LLVM_TESTING_ADT_STRINGMAP_H_
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Testing/ADT/StringMapEntry.h"
+#include <ostream>
+#include <sstream>
+
+namespace llvm {
+
+/// Support for printing to std::ostream, for use with e.g. producing more
+/// useful error messages with Google Test.
+template <typename T>
+std::ostream &operator<<(std::ostream &OS, const StringMap<T> &M) {
+  if (M.empty()) {
+    return OS << "{ }";
+  }
+
+  std::vector<std::string> Lines;
+  for (const auto &E : M) {
+    std::ostringstream SS;
+    SS << E << ",";
+    Lines.push_back(SS.str());
+  }
+  llvm::sort(Lines);
+  Lines.insert(Lines.begin(), "{");
+  Lines.insert(Lines.end(), "}");
+
+  return OS << llvm::formatv("{0:$[\n]}",
+                             make_range(Lines.begin(), Lines.end()))
+                   .str();
+}
+
+} // namespace llvm
+
+#endif

llvm/include/llvm/Testing/ADT/StringMapEntry.h

+//===- llvm/Testing/ADT/StringMapEntry.h ----------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TESTING_ADT_STRINGMAPENTRY_H_
+#define LLVM_TESTING_ADT_STRINGMAPENTRY_H_
+
+#include "llvm/ADT/StringMapEntry.h"
+#include <ostream>
+#include <type_traits>
+
+namespace llvm {
+namespace detail {
+
+template <typename T, typename = std::void_t<>>
+struct CanOutputToOStream : std::false_type {};
+
+template <typename T>
+struct CanOutputToOStream<T, std::void_t<decltype(std::declval<std::ostream &>()
+                                                  << std::declval<T>())>>
+    : std::true_type {};
+
+} // namespace detail
+
+/// Support for printing to std::ostream, for use with e.g. producing more
+/// useful error messages with Google Test.
+template <typename T>
+std::ostream &operator<<(std::ostream &OS, const StringMapEntry<T> &E) {
+  OS << "{\"" << E.getKey().data() << "\": ";
+  if constexpr (detail::CanOutputToOStream<decltype(E.getValue())>::value) {
+    OS << E.getValue();
+  } else {
+    OS << "non-printable value";
+  }
+  return OS << "}";
+}
+
+} // namespace llvm
+
+#endif

llvm/unittests/Testing/ADT/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_llvm_unittest(TestingADTTests
+  StringMapTest.cpp
+  )

llvm/unittests/Testing/ADT/StringMapTest.cpp

+//===- StringMapTest.cpp --------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Testing/ADT/StringMap.h"
+#include "llvm/ADT/StringMap.h"
+
+#include "gtest/gtest.h"
+#include <sstream>
+
+namespace llvm {
+namespace {
+
+TEST(StringMapTest, StringMapStream) {
+  std::ostringstream OS;
+  StringMap<int> Map;
+  Map["A"] = 42;
+  Map["Z"] = 35;
+  Map["B"] = 7;
+  OS << Map;
+
+  EXPECT_EQ(OS.str(), R"({
+{"A": 42},
+{"B": 7},
+{"Z": 35},
+})");
+}
+
+TEST(StringMapTest, NestedStringMapStream) {
+  std::ostringstream OS;
+  StringMap<StringMap<int>> Map;
+  Map["Z"];
+  Map["A"]["Apple"] = 5;
+  Map["B"]["Bee"] = 3;
+  Map["A"]["Axe"] = 3;
+  OS << Map;
+
+  EXPECT_EQ(OS.str(), R"({
+{"A": {
+{"Apple": 5},
+{"Axe": 3},
+}},
+{"B": {
+{"Bee": 3},
+}},
+{"Z": { }},
+})");
+}
+
+} // namespace
+} // namespace llvm

llvm/unittests/Testing/CMakeLists.txt

+add_subdirectory(ADT)
 add_subdirectory(Support)

utils/bazel/llvm-project-overlay/llvm/BUILD.bazel

 
 py_binary(
     name = "lit",
     testonly = True,
     srcs = ["utils/lit/lit.py"] + glob(["utils/lit/lit/**/*.py"]),
 )
 
 cc_library(
+    name = "TestingADT",
+    testonly = True,
+    hdrs = glob([
+        "include/llvm/Testing/ADT/*.h",
+    ]),
+    copts = llvm_copts,
+    deps = [":Support"],
+)
+
+cc_library(
     name = "TestingSupport",
     testonly = True,
     srcs = glob([
         "lib/Testing/Support/*.cpp",
         "lib/Testing/Support/*.h",
     ]),
     hdrs = glob([
         "include/llvm/Testing/Support/*.h",

utils/bazel/llvm-project-overlay/llvm/unittests/BUILD.bazel

cc_test(
     deps = [
         "//llvm:AArch64CodeGen",
         "//llvm:gtest",
         "//llvm:gtest_main",
     ],
 )
 
 cc_test(
+    name = "testing_adt_tests",
+    size = "small",
+    srcs = glob(
+        [
+            "Testing/ADT/*.cpp",
+        ],
+        allow_empty = False,
+    ),
+    deps = [
+        "//llvm:Support",
+        "//llvm:TestingADT",
+        "//llvm:gtest",
+        "//llvm:gtest_main",
+    ],
+)
+
+cc_test(
     name = "transforms_tests",
     size = "small",
     srcs = glob(
         [
             "Transforms/IPO/*.cpp",
             "Transforms/IPO/*.h",
             "Transforms/Utils/*.cpp",
         ],