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

[Yurong via Phabricator via libcxx-commits]

yronglin added inline comments.


================
Comment at: libcxx/include/__expected/expected.h:843
+    using _Gp = remove_cv_t<invoke_result_t<_Func, _Err&&>>;
+    if constexpr (!__valid_std_unexpected<_Gp>::value) {
+      static_assert(__valid_std_unexpected<_Gp>::value,
----------------
ldionne wrote:
> yronglin wrote:
> > ldionne wrote:
> > > yronglin wrote:
> > > > So sorry, this code looks very ugly, it used to simplify the compile error diagnostic message(useful when I write the test case to check the `static_assert message` for LWG3866), do you have more good ideas for this? 
> > > > 
> > > > before:
> > > > ```
> > > > In file included from /Users/yrong/Developer/Toolchain/llvm/trunk/llvm-project/libcxx/test/libcxx/utilities/expected/expected.void/transform_error.mandates.verify.cpp:29:
> > > > In file included from /Users/yrong/Developer/Toolchain/llvm/trunk/rel/include/c++/v1/expected:44:
> > > > /Users/yrong/Developer/Toolchain/llvm/trunk/rel/include/c++/v1/__expected/expected.h:1433:19: error: static assertion failed due to requirement 'integral_constant<bool, false>::value': The result of f(error()) must be a valid template argument for unexpected
> > > >     static_assert(__valid_std_unexpected<_Gp>::value,
> > > >                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > /Users/yrong/Developer/Toolchain/llvm/trunk/llvm-project/libcxx/test/libcxx/utilities/expected/expected.void/transform_error.mandates.verify.cpp:50:7: note: in instantiation of function template specialization 'std::expected<void, int>::transform_error<std::unexpected<int> (&)(int &)>' requested here
> > > >     e.transform_error(return_unexpected); 
> > > >       ^
> > > > In file included from /Users/yrong/Developer/Toolchain/llvm/trunk/llvm-project/libcxx/test/libcxx/utilities/expected/expected.void/transform_error.mandates.verify.cpp:29:
> > > > In file included from /Users/yrong/Developer/Toolchain/llvm/trunk/rel/include/c++/v1/expected:44:
> > > > /Users/yrong/Developer/Toolchain/llvm/trunk/rel/include/c++/v1/__expected/expected.h:954:17: error: static assertion failed due to requirement 'integral_constant<bool, false>::value': [expected.void.general] A program that instantiates expected<T, E> with a E that is not a valid argument for unexpected<E> is ill-formed
> > > >   static_assert(__valid_std_unexpected<_Err>::value,
> > > >                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > /Users/yrong/Developer/Toolchain/llvm/trunk/rel/include/c++/v1/__expected/expected.h:1436:14: note: in instantiation of template class 'std::expected<void, std::unexpected<int>>' requested here
> > > >       return expected<_Tp, _Gp>();
> > > >              ^
> > > > /Users/yrong/Developer/Toolchain/llvm/trunk/llvm-project/libcxx/test/libcxx/utilities/expected/expected.void/transform_error.mandates.verify.cpp:50:7: note: in instantiation of function template specialization 'std::expected<void, int>::transform_error<std::unexpected<int> (&)(int &)>' requested here
> > > >     e.transform_error(return_unexpected); 
> > > >       ^
> > > > In file included from /Users/yrong/Developer/Toolchain/llvm/trunk/llvm-project/libcxx/test/libcxx/utilities/expected/expected.void/transform_error.mandates.verify.cpp:29:
> > > > In file included from /Users/yrong/Developer/Toolchain/llvm/trunk/rel/include/c++/v1/expected:44:
> > > > /Users/yrong/Developer/Toolchain/llvm/trunk/rel/include/c++/v1/__expected/expected.h:1438:12: error: excess elements in struct initializer
> > > >     return expected<_Tp, _Gp>(__expected_construct_unexpected_from_invoke_tag{}, std::forward<_Func>(__f), error());
> > > >            ^                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > /Users/yrong/Developer/Toolchain/llvm/trunk/llvm-project/libcxx/test/libcxx/utilities/expected/expected.void/transform_error.mandates.verify.cpp:50:7: note: in instantiation of function template specialization 'std::expected<void, int>::transform_error<std::unexpected<int> (&)(int &)>' requested here
> > > >     e.transform_error(return_unexpected); 
> > > >       ^
> > > > In file included from /Users/yrong/Developer/Toolchain/llvm/trunk/llvm-project/libcxx/test/libcxx/utilities/expected/expected.void/transform_error.mandates.verify.cpp:29:
> > > > In file included from /Users/yrong/Developer/Toolchain/llvm/trunk/rel/include/c++/v1/expected:44:
> > > > ```
> > > > after:
> > > > ```
> > > > In file included from /Users/yrong/Developer/Toolchain/llvm/trunk/llvm-project/libcxx/test/libcxx/utilities/expected/expected.expected/transform_error.mandates.verify.cpp:29:
> > > > In file included from /Users/yrong/Developer/Toolchain/llvm/trunk/rel/include/c++/v1/expected:44:
> > > > /Users/yrong/Developer/Toolchain/llvm/trunk/rel/include/c++/v1/__expected/expected.h:814:19: error: static assertion failed due to requirement 'integral_constant<bool, false>::value': The result of f(error()) must be a valid template argument for unexpected
> > > >     static_assert(__valid_std_unexpected<_Gp>::value,
> > > >                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > /Users/yrong/Developer/Toolchain/llvm/trunk/llvm-project/libcxx/test/libcxx/utilities/expected/expected.expected/transform_error.mandates.verify.cpp:45:7: note: in instantiation of function template specialization 'std::expected<int, int>::transform_error<std::unexpected<int> (&)(int)>' requested here
> > > >     e.transform_error(return_unexpected<int&>); // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}} The result of {{.*}} must be a valid template argument for unexpected}}
> > > >       ^
> > > > In file included from /Users/yrong/Developer/Toolchain/llvm/trunk/llvm-project/libcxx/test/libcxx/utilities/expected/expected.expected/transform_error.mandates.verify.cpp:29:
> > > > In file included from /Users/yrong/Developer/Toolchain/llvm/trunk/rel/include/c++/v1/expected:44:
> > > > /Users/yrong/Developer/Toolchain/llvm/trunk/rel/include/c++/v1/__expected/expected.h:814:19: error: static assertion failed due to requirement 'integral_constant<bool, false>::value': The result of f(error()) must be a valid template argument for unexpected
> > > >     static_assert(__valid_std_unexpected<_Gp>::value,
> > > >                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > /Users/yrong/Developer/Toolchain/llvm/trunk/llvm-project/libcxx/test/libcxx/utilities/expected/expected.expected/transform_error.mandates.verify.cpp:46:7: note: in instantiation of function template specialization 'std::expected<int, int>::transform_error<int &(&)(int)>' requested here
> > > >     e.transform_error(return_no_object<int&>); // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}} The result of {{.*}} must be a valid template argument for unexpected}}
> > > >       ^
> > > > In file included from /Users/yrong/Developer/Toolchain/llvm/trunk/llvm-project/libcxx/test/libcxx/utilities/expected/expected.expected/transform_error.mandates.verify.cpp:29:
> > > > In file included from /Users/yrong/Developer/Toolchain/llvm/trunk/rel/include/c++/v1/expected:44:
> > > > /Users/yrong/Developer/Toolchain/llvm/trunk/rel/include/c++/v1/__expected/expected.h:829:19: error: static assertion failed due to requirement 'integral_constant<bool, false>::value': The result of f(error()) must be a valid template argument for unexpected
> > > >     static_assert(__valid_std_unexpected<_Gp>::value,
> > > >                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > /Users/yrong/Developer/Toolchain/llvm/trunk/llvm-project/libcxx/test/libcxx/utilities/expected/expected.expected/transform_error.mandates.verify.cpp:52:7: note: in instantiation of function template specialization 'std::expected<int, int>::transform_error<std::unexpected<int> (&)(int)>' requested here
> > > >     e.transform_error(return_unexpected<const int &>); // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}} The result of {{.*}} must be a valid template argument for unexpected}}
> > > >       ^
> > > > 
> > > > ```
> > > I'm not sure I see how the "after" is better than the "before"? They look very similar, don't they?
> > The "before" 1 case emit 3 errors, but "after" 1 case only emit 1 static assertion error.
> IMO either way is acceptable. The real culprit is that Clang tries to keep compiling and that's really hard to work around that. For example with your current approach, the function will now return `void` and that could lead to other diagnostics.
Thanks for your tips, I finally remove this approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140911