[libcxx-commits] [PATCH] D114761: [libc++][ranges] Implement [special.mem.concepts].

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/test/std/algorithms/specialized.algorithms/special.mem.concepts/special.mem.concepts.compile.pass.cpp:49-52
+// __nothrow_sentinel_for
+
+static_assert(std::ranges::__nothrow_sentinel_for<int*, int*>);
+static_assert(!std::ranges::__nothrow_sentinel_for<int*, long*>);
----------------
ldionne wrote:
> ldionne wrote:
> > jloser wrote:
> > > var-const wrote:
> > > > Quuxplusone wrote:
> > > > > For this one, I think it would be reasonable to test that `__nothrow_sentinel_for` subsumes `sentinel_for`, //and// vice versa. However, that might also be overkill. Untested code:
> > > > > ```
> > > > > bool ntsf_subsumes_sf(__nothrow_sentinel_for<char*> auto) requires true { return true; }
> > > > > bool ntsf_subsumes_sf(sentinel_for<char*> auto);
> > > > > 
> > > > > bool sf_subsumes_ntsf(sentinel_for<char*> auto) requires true { return true; }
> > > > > bool sf_subsumes_ntsf(__nothrow_sentinel_for<char*> auto);
> > > > > 
> > > > > assert(ntsf_subsumes_sf("foo"));
> > > > > assert(sf_subsumes_ntsf("foo"));
> > > > > ```
> > > > Honestly, it seems to me like it's a little overkill. The Standard specifies `nothrow-sentinel-for` to be essentially an alias for `sentinel_for`, and it doesn't really make sense to implement it any other way.
> > > I like the general idea of testing subsumption for concepts, including these. Overkill for most user concepts in codebases, but for standard library implementors, I think it's reasonable. WDYT, @ldionne @cjdb?
> > We did some of this at some point, but as concepts become larger and more complex, it ends up being incredibly complicated to do that. You basically end up copy-pasting exactly the sub-expressions that are used in the implementation to make sure that the concept subsumes these sub-expressions. I'm not convinced that it provides that much bang for our buck, except in simple cases (e.g. it's obvious that `forward_iterator` should subsume `input_iterator`, and that should be checked).
> So yeah, writing that, I guess it makes sense to check that `nothrow_forward_iterator` subsumes `nothrow_input_iterator`.
I'd prefer to merge this patch as-is and add subsumption tests in a follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114761