[libcxx-commits] [PATCH] D140911: [libc++] Implement P2505R5(Monadic operations for std::expected)

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__expected/expected.h:68
+
 namespace __expected {
 
----------------
huixie90 wrote:
> philnik wrote:
> > @huixie90 Why do we have this namespace? It doesn't really seem worth it just for `__throw_bad_expected_access`.
> The honest answer is i can't remember. perhaps in some point of time I had several functions but got deleted in the end. But it seems that after this patch, there is more stuff in it.
I would remove the namespace then. It doesn't really improve the readability in any way, since every single member has `expected` already in it's name. We also don't have these kinds of detail namespaces normally.


================
Comment at: libcxx/test/libcxx/utilities/expected/expected.expected/and_then.mandates.verify.cpp:10
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+// ADDITIONAL_COMPILE_FLAGS: -Xclang -verify-ignore-unexpected=error
+
----------------
This seems weird. Why should we ignore errors?


================
Comment at: libcxx/test/std/utilities/expected/expected.expected/monadic/and_then.pass.cpp:79
+
+struct NOLVal {
+  constexpr std::expected<int, int> operator()(int&) { return std::expected<int, int>(std::unexpected<int>(5)); }
----------------



================
Comment at: libcxx/test/std/utilities/expected/expected.expected/monadic/and_then.pass.cpp:154
+
+static_assert( has_and_then<std::expected<int, int>&, decltype([](int&)->std::expected<int, int> { return 0; })>);
+static_assert(!has_and_then<std::expected<int, NonCopyable>&, decltype([](int&)->std::expected<int, NonCopyable> { return 0; })>);
----------------
Same for the others.


================
Comment at: libcxx/test/std/utilities/expected/expected.expected/monadic/and_then.pass.cpp:169-172
+      std::expected<int, int> e{0};
+      assert(e.and_then(LVal{}).value() == 1);
+      assert(e.and_then(NOLVal{}).error() == 5);
+      static_assert(std::is_same_v<decltype(e.and_then(LVal{})), std::expected<int, int>>);
----------------



================
Comment at: libcxx/test/std/utilities/expected/expected.void/observers/error_or.pass.cpp:93
+  test_default_template_arg();
+  static_assert(test_default_template_arg());
+
----------------
Why is this only `static_assert`ed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140911