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

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

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/algorithms/specialized.algorithms/special.mem.concepts/special.mem.concepts.compile.pass.cpp:38-42
+template <typename T = int>
+struct InputIterator_Proxy : MoveOnly, Incrementable<InputIterator_Proxy<T>>, HasIteratorConcept<> {
+  using value_type = T;
+  value_type operator*() const;
+};
----------------
var-const wrote:
> Quuxplusone wrote:
> > I believe this should be more like https://godbolt.org/z/PTce8axh5 :
> > ```
> > bool is_nothrow_input_iterator(std::ranges::__nothrow_input_iterator auto) { return true; }
> > bool is_nothrow_input_iterator(std::input_iterator auto) { return false; }
> > 
> > int main() {
> >     assert(!is_nothrow_input_iterator(InputIt{}));
> >     assert(is_nothrow_input_iterator(NothrowInputIt{}));
> > }
> > ```
> > Also please take the `NothrowInputIt` and `InputIt` from that godbolt; they're pretty minimal.
> What's the advantage of using these functions? They add a layer of abstraction and, AFAICT, make this a runtime test rather than a compile-time test. Also, why define a custom `InputIt` when `test_iterators.h` already contains suitable classes?
Overloading like this is the pattern for testing subsumption relationships. The Standard says that If you have an overload set where one overload is constrained on `__nothrow_input_iterator` and another is constrained only on `input_iterator`, they won't be ambiguous overloads. So in general, when a subsumption relationship is mandated, we ought to have a test following this pattern somewhere.

However, in writing this comment, I realized that the Stupid C++ Trick I was thinking this enabled, isn't even legal. I was thinking that a sufficiently evil user could write https://godbolt.org/z/E4WT434xa
```
struct NotDC {
    NotDC(int) {}
    void uninitialized_default_construct(std::ranges::forward_range auto&& r) {}
};
int main() {
    NotDC a[3] = {1, 2, 3};
    using std::ranges::uninitialized_default_construct;
    uninitialized_default_construct(a);
}
```
But in fact subsumption is completely irrelevant to that code. It won't work because `std::ranges::uninitialized_default_construct` is a niebloid and so there is no overload set here at all. Conclusion: the subsumption relationship //is// mandated, but I no longer think that any user could possibly observe it, so we probably don't need to test it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114761