[libcxx-commits] [PATCH] D102434: [libcxx][ranges] adds concept `sized_range` and cleans up `ranges::size`

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/strings/string.view/range_concept_conformance.compile.pass.cpp:31
 static_assert(stdr::random_access_range<std::string_view const>);
 static_assert(!stdr::view<std::string_view const>);
+static_assert(stdr::sized_range<std::string_view const>);
----------------
zoecarver wrote:
> cjdb wrote:
> > Quuxplusone wrote:
> > > Unrelated to this PR, but it strikes me as odd that `string_view` is //not// a `std::ranges::view`.
> > > Should you perhaps be asserting `stdr::view<std::string_view&>` instead?
> > That would be because we haven't enabled `string_view` as a view yet.
> Hmm. I think we should at least have a comment saying that this test is wrong. I'd even say we should comment out the static_assert too (and make it positive). 
@zoecarver: If we comment out the assert, we'll have to set some kind of watchdog to remind us to comment it back in, in the PR that enables `string_view`-as-a-view. Which will definitely fail to remind us, and then tech debt.
By keeping this "wrong" assert enabled, it serves as a regression test: it serves as its own infallible watchdog. When `string_view` becomes a view, buildkite will point //directly at this test file//, telling us that we need to flip the sense of the assert as part of the same PR. That's excellent. (And also excellent to see in the git history later, when we're wondering which commit caused `string_view` to become a view.)
I wouldn't mind seeing a `// TODO: string_view is nonconforming` comment at the end of this line, though.

A possible alternative approach (but I assume we won't block D102434 on this): //first// land a PR that makes `string_view`-a-view; //then// land this PR with the assert already passing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102434