[libcxx-commits] [PATCH] D117714: [libc++] Implement LWG3549: view_interface need not inherit from view_base
Arthur O'Dwyer via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Jan 19 17:49:35 PST 2022
Quuxplusone added a comment.
Probably you should add a libcxx/test/libcxx/ test that proves the common `view_base` base is gone. Something like
struct V1 : view_interface<V1> { };
struct V2 : view_interface<V2> { V1 base_; };
static_assert(sizeof(V2) == sizeof(V1));
(check via Godbolt that this fails today, of course; and add member functions if needed but I think they're actually not needed)
================
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;
----------------
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.)
================
Comment at: libcxx/include/__ranges/view_interface.h:38-39
template<class _Tp>
void __implicitly_convert_to(type_identity_t<_Tp>) noexcept;
----------------
Pre-existing: I bet we reinvent this helper all over the place. We should centralize it in <type_traits>.
(No action required here.)
================
Comment at: libcxx/include/__ranges/view_interface.h:76
constexpr explicit operator bool()
noexcept(noexcept(ranges::empty(declval<_D2>())))
requires __can_empty<_D2>
----------------
Pre-existing: The repeated typo `_D2` for `_D2&` suggests to me that our test coverage for `view_interface` is somewhat insufficient.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117714/new/
https://reviews.llvm.org/D117714
More information about the libcxx-commits
mailing list