[PATCH] D105014: added some example code for llvm::Expected<T>

[Sam McCall via Phabricator via llvm-commits]

sammccall added inline comments.


================
Comment at: llvm/include/llvm/Testing/Support/Error.h:168
 
+/// Helper marcro for checking the result of an 'Expected<T>'
+///
----------------
marcro -> macro


================
Comment at: llvm/include/llvm/Testing/Support/Error.h:179
+///       // calling 'toString()'. This also helps in debugging failing tests.
+///       EXPECT_THAT_EXPECTED(D1, Succeeded()) << toString(D1.takeError()) <<
+///         "\n";
----------------
you shouldn't explicitly log the tostring, this should happen automatically


================
Comment at: llvm/include/llvm/Testing/Support/Error.h:181
+///         "\n";
+///       EXPECT_THAT(*D1, Eq(2));
+///
----------------
this is unidiomatic/incorrect, rather just `EXPECT_THAT_EXPECTED(D1, HasValue(2))` in one step

(Note that if you want to write `*D1`, you need to establish that `D1` succeeded, e.g. by using `ASSERT_` to bail out early. As it is, if there is an error it'll show one failure and then crash)


================
Comment at: llvm/include/llvm/Testing/Support/Error.h:187
+///       // In the error case we need to consume the error.
+///       consumeError(D2.takeError());
+///     }
----------------
no, you don't!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105014/new/

https://reviews.llvm.org/D105014