As discussed in https://reviews.llvm.org/D117714, there is missing test coverage
for the behavior of enable_view when given a const or reference qualified
type. Add such tests showing the current behavior.
Details
- Reviewers
• Quuxplusone ldionne Mordante var-const philnik - Group Reviewers
Restricted Project - Commits
- rG4f547ee8b8a7: [libc++][test] Add const and reference tests for enable_view. NFC.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Thanks! This is an improvement as-is, but I think you should consider merging all your new tests from D117714 into this review. They should all work today, because view_interface<T> already inherits from view_base. Then your D117714 will become a code-only change, with no diffs to any of the tests except for the empty-base-sizeof test (and I guess if merging the tests over here discovers any bugs in the existing implementation!).
libcxx/test/std/ranges/range.req/range.view/enable_view.compile.pass.cpp | ||
---|---|---|
46 | Exactly my thoughts as well. It's clear from a technical perspective, but surely this isn't what the user intended. They likely intended to disable cv-qual of their type as well and expected us to do it for them basically by opting in for their type only (and not aalso Generally speaking, this is likely to come up with other customization points in the standard library, but I'd have to search for other "broken" ones. |
libcxx/test/std/ranges/range.req/range.view/enable_view.compile.pass.cpp | ||
---|---|---|
46 | Initial reply from LWG is: "Who cares?" The only person who should be querying enable_view<X> is concept view itself; no user-programmer should ever be querying it. And concept view will care about enable_view<X> only when movable<X>, which means
Here are the test cases from my attempted-LWG-issue-filing, for the record. struct V1 : std::ranges::view_base {}; struct V2 : std::ranges::view_interface<V2> {}; struct V3 {}; template<> constexpr bool std::ranges::enable_view<V3> = true; struct V4 : std::view_base {}; template<> constexpr bool std::ranges::enable_view<V4> = false; static_assert(std::ranges::enable_view<V1> == true); static_assert(std::ranges::enable_view<V2> == true); static_assert(std::ranges::enable_view<V3> == true); static_assert(std::ranges::enable_view<V4> == false); static_assert(std::ranges::enable_view<const V1> == true); LIBCPP_STATIC_ASSERT(std::ranges::enable_view<const V2> == true); static_assert(std::ranges::enable_view<const V3> == false); static_assert(std::ranges::enable_view<const V4> == true); |