[Testing/Support] Make matchers work with Expected<T&>

Summary:
This did not work because the ExpectedHolder was trying to hold the
value in an Optional<T*>. Instead of trying to mimic the behavior of
Expected and try to make ExpectedHolder work with references and
non-references, I simply store the reference to the Expected object in
the holder.

I also add a bunch of tests for these matchers, which have helped me
flesh out some problems in my initial implementation of this patch, and
uncovered the fact that we are not consistent in quoting our values in
the matcher output (which I also fix).

Reviewers: zturner, chandlerc

Subscribers: mgorny, llvm-commits

Differential Revision: https://reviews.llvm.org/D40904

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@320025 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Testing/Support/Error.h

@@ -22,12 +22,7 @@ namespace detail {
 ErrorHolder TakeError(Error Err);
 
 template <typename T> ExpectedHolder<T> TakeExpected(Expected<T> &Exp) {
-  llvm::detail::ExpectedHolder<T> Result;
-  auto &EH = static_cast<llvm::detail::ErrorHolder &>(Result);
-  EH = TakeError(Exp.takeError());
-  if (Result.Success)
-    Result.Value = &(*Exp);
-  return Result;
+  return {TakeError(Exp.takeError()), Exp};
 }
 
 template <typename T> ExpectedHolder<T> TakeExpected(Expected<T> &&Exp) {
@@ -49,16 +44,15 @@ MATCHER(Succeeded, "") { return arg.Success; }
 MATCHER(Failed, "") { return !arg.Success; }
 
 MATCHER_P(HasValue, value,
-          "succeeded with value " + testing::PrintToString(value)) {
+          "succeeded with value \"" + testing::PrintToString(value) + '"') {
   if (!arg.Success) {
     *result_listener << "operation failed";
     return false;
   }
 
-  assert(arg.Value.hasValue());
-  if (**arg.Value != value) {
-    *result_listener << "but \"" + testing::PrintToString(**arg.Value) +
-                            "\" != " + testing::PrintToString(value);
+  if (*arg.Exp != value) {
+    *result_listener << "but \"" + testing::PrintToString(*arg.Exp) +
+                            "\" != \"" + testing::PrintToString(value) + '"';
     return false;
   }
 

include/llvm/Testing/Support/SupportHelpers.h

@@ -22,7 +22,10 @@ struct ErrorHolder {
 };
 
 template <typename T> struct ExpectedHolder : public ErrorHolder {
-  Optional<T *> Value;
+  ExpectedHolder(ErrorHolder Err, Expected<T> &Exp)
+      : ErrorHolder(std::move(Err)), Exp(Exp) {}
+
+  Expected<T> &Exp;
 };
 
 inline void PrintTo(const ErrorHolder &Err, std::ostream *Out) {
@@ -35,7 +38,7 @@ inline void PrintTo(const ErrorHolder &Err, std::ostream *Out) {
 template <typename T>
 void PrintTo(const ExpectedHolder<T> &Item, std::ostream *Out) {
   if (Item.Success) {
-    *Out << "succeeded with value \"" << ::testing::PrintToString(**Item.Value)
+    *Out << "succeeded with value \"" << ::testing::PrintToString(*Item.Exp)
          << "\"";
   } else {
     PrintTo(static_cast<const ErrorHolder &>(Item), Out);

unittests/Support/CMakeLists.txt

@@ -68,6 +68,8 @@ add_llvm_unittest(SupportTests
   xxhashTest.cpp
   )
 
+target_link_libraries(SupportTests PRIVATE LLVMTestingSupport)
+
 # Disable all warning for AlignOfTest.cpp,
 # as it does things intentionally, and there is no reliable way of
 # disabling all warnings for all the compilers by using pragmas.

unittests/Support/ErrorTest.cpp

@@ -12,6 +12,8 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest-spi.h"
 #include "gtest/gtest.h"
 #include <memory>
 
@@ -714,4 +716,42 @@ TEST(Error, ErrorMessage) {
             0);
 }
 
+TEST(Error, ErrorMatchers) {
+  EXPECT_THAT_ERROR(Error::success(), Succeeded());
+  EXPECT_NONFATAL_FAILURE(
+      EXPECT_THAT_ERROR(make_error<CustomError>(0), Succeeded()),
+      "Expected: succeeded\n  Actual: failed  (CustomError { 0})");
+
+  EXPECT_THAT_ERROR(make_error<CustomError>(0), Failed());
+  EXPECT_NONFATAL_FAILURE(EXPECT_THAT_ERROR(Error::success(), Failed()),
+                          "Expected: failed\n  Actual: succeeded");
+
+  EXPECT_THAT_EXPECTED(Expected<int>(0), Succeeded());
+  EXPECT_NONFATAL_FAILURE(
+      EXPECT_THAT_EXPECTED(Expected<int>(make_error<CustomError>(0)),
+                           Succeeded()),
+      "Expected: succeeded\n  Actual: failed  (CustomError { 0})");
+
+  EXPECT_THAT_EXPECTED(Expected<int>(make_error<CustomError>(0)), Failed());
+  EXPECT_NONFATAL_FAILURE(
+      EXPECT_THAT_EXPECTED(Expected<int>(0), Failed()),
+      "Expected: failed\n  Actual: succeeded with value \"0\"");
+
+  EXPECT_THAT_EXPECTED(Expected<int>(0), HasValue(0));
+  EXPECT_NONFATAL_FAILURE(
+      EXPECT_THAT_EXPECTED(Expected<int>(make_error<CustomError>(0)),
+                           HasValue(0)),
+      "Expected: succeeded with value \"0\"\n"
+      "  Actual: failed  (CustomError { 0})");
+  EXPECT_NONFATAL_FAILURE(
+      EXPECT_THAT_EXPECTED(Expected<int>(1), HasValue(0)),
+      "Expected: succeeded with value \"0\"\n"
+      "  Actual: succeeded with value \"1\", but \"1\" != \"0\"");
+
+  EXPECT_THAT_EXPECTED(Expected<int &>(make_error<CustomError>(0)), Failed());
+  int a = 1;
+  EXPECT_THAT_EXPECTED(Expected<int &>(a), Succeeded());
+  EXPECT_THAT_EXPECTED(Expected<int &>(a), HasValue(1));
+}
+
 } // end anon namespace