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

[Yurong via Phabricator via libcxx-commits]

yronglin marked an inline comment as done.
yronglin added inline comments.


================
Comment at: libcxx/include/__expected/expected.h:68
+
 namespace __expected {
 
----------------
philnik wrote:
> yronglin wrote:
> > yronglin wrote:
> > > yronglin wrote:
> > > > philnik wrote:
> > > > > 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.
> > > > If we decided to remove __expected namespace, I think __unexpected namespace in unexpected.h also should be removed, What do you think?
> > > If we decided to remove `__expected` namespace, I think `__unexpected` namespace in unexpected.h also should be removed, What do you think?
> > Seems this namespace used to avoid ADL lookup?
> > ```
> > $ ":" "RUN: at line 15"
> > note: command had no output on stdout or stderr
> > $ "clang-tidy-16" "/home/libcxx-builder/.buildkite-agent/builds/8ae0e84df61e-1/llvm-project/libcxx-ci/libcxx/test/libcxx/clang_tidy.sh.cpp" "--warnings-as-errors=*" "-header-filter=.*" "--checks=-*,libcpp-*" "--load=/home/libcxx-builder/.buildkite-agent/builds/8ae0e84df61e-1/llvm-project/libcxx-ci/build/generic-cxx2b/libcxx/test/tools/clang_tidy_checks/libcxx-tidy.plugin" "--" "-nostdinc++" "-I" "/home/libcxx-builder/.buildkite-agent/builds/8ae0e84df61e-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1" "-I" "/home/libcxx-builder/.buildkite-agent/builds/8ae0e84df61e-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1" "-I" "/home/libcxx-builder/.buildkite-agent/builds/8ae0e84df61e-1/llvm-project/libcxx-ci/libcxx/test/support" "-std=c++2b" "-Werror" "-Wall" "-Wctad-maybe-unsupported" "-Wextra" "-Wshadow" "-Wundef" "-Wno-unused-command-line-argument" "-Wno-attributes" "-Wno-pessimizing-move" "-Wno-c++11-extensions" "-Wno-noexcept-type" "-Wno-atomic-alignment" "-Wno-user-defined-literals" "-Wno-tautological-compare" "-Wsign-compare" "-Wunused-variable" "-Wunused-parameter" "-Wunreachable-code" "-Wno-unused-local-typedef" "-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER" "-D_LIBCPP_ENABLE_EXPERIMENTAL" "-D_LIBCPP_DISABLE_AVAILABILITY" "-fcoroutines-ts" "-Werror=thread-safety" "-Wuser-defined-warnings" "-fno-modules"
> > # command output:
> > /home/libcxx-builder/.buildkite-agent/builds/8ae0e84df61e-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1/__expected/expected.h:552:7: error: ADL lookup [libcpp-robust-against-adl,-warnings-as-errors]
> >       __throw_bad_expected_access<_Err>(__union_.__unex_);
> >       ^
> > /home/libcxx-builder/.buildkite-agent/builds/8ae0e84df61e-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1/__expected/expected.h:559:7: error: ADL lookup [libcpp-robust-against-adl,-warnings-as-errors]
> >       __throw_bad_expected_access<_Err>(__union_.__unex_);
> >       ^
> > /home/libcxx-builder/.buildkite-agent/builds/8ae0e84df61e-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1/__expected/expected.h:566:7: error: ADL lookup [libcpp-robust-against-adl,-warnings-as-errors]
> >       __throw_bad_expected_access<_Err>(std::move(__union_.__unex_));
> >       ^
> > /home/libcxx-builder/.buildkite-agent/builds/8ae0e84df61e-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1/__expected/expected.h:573:7: error: ADL lookup [libcpp-robust-against-adl,-warnings-as-errors]
> >       __throw_bad_expected_access<_Err>(std::move(__union_.__unex_));
> >       ^
> > /home/libcxx-builder/.buildkite-agent/builds/8ae0e84df61e-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1/__expected/expected.h:1167:7: error: ADL lookup [libcpp-robust-against-adl,-warnings-as-errors]
> >       __throw_bad_expected_access<_Err>(__union_.__unex_);
> >       ^
> > /home/libcxx-builder/.buildkite-agent/builds/8ae0e84df61e-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1/__expected/expected.h:1173:7: error: ADL lookup [libcpp-robust-against-adl,-warnings-as-errors]
> >       __throw_bad_expected_access<_Err>(std::move(__union_.__unex_));
> >       ^
> > 
> > # command stderr:
> > 6 warnings generated.
> > Suppressed 48 warnings (48 NOLINT).
> > 6 warnings treated as errors
> > 
> > error: command failed with exit status: 1
> > 
> > ```
> Yes, I think we should also remove `__unexpected`, but let's do that in another patch. You have to add `std::` then calling `__throw_bad_expected_access` now to avoid the ADL lookup.
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140911