[libcxx-commits] [PATCH] D117714: [libc++] Implement LWG3549: view_interface need not inherit from view_base

Thu Jan 20 15:24:45 PST 2022

jloser added inline comments.

================
Comment at: libcxx/include/__ranges/enable_view.h:36
 template <class _Tp>
-inline constexpr bool enable_view = derived_from<_Tp, view_base>;
+inline constexpr bool enable_view = derived_from<_Tp, view_base> || decltype(__inherits_from_view_interface(static_cast<remove_cvref_t<_Tp>*>(nullptr)))::value;

----------------
Quuxplusone wrote:
> Quuxplusone wrote:
> > Please add an ADL-proofing regression test that will catch the lack of qualification on `__inherits_from_view_interface`; and then change it to `_VSTD::__inherits_from_view_interface` so that the test case passes. (Grep for `Holder<Incomplete>` to see our traditional ADL trap.)
> FYI, you don't need (and therefore shouldn't have) qualification on non-functions. One level of qualification is needed purely to turn off ADL, and for no other purpose. So: `ranges::__derived_from_view_interface_helper(...)`, `__is_derived_from_view_interface<...>`.
> However, this //still// feels crazy complicated. Why isn't this just the-thing-I-posted-yesterday with the names uglified and ADL-proofed? Why does it need inline constexpr variables //and// concepts //and// partial specializations //and// the-thing-I-said?
We can fix up the qualifications, no problem.

It does feel more complicated than it needs to be - I'll play with it some more maybe later tonight. The thing we need to jump through some hoops for is to avoid `static_cast<T>(nullptr)` being a hard error (which is the case for when privately inheriting from `view_interface`). In this case, the number of hoops is more than (you or I) like.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117714