[libcxx-commits] [PATCH] D115315: [libc++][ranges] Implement ranges::uninitialized_default_construct{, _n}.

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

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.default/special_function.compile.pass.cpp:24-55
+namespace found_by_adl {
+
+template <class I = int*>
+struct adl_forward_iterator : forward_iterator<I> {
+  // To satisfy `weakly_incrementable`, the increment operators must return the same type as the
+  // class.
+  adl_forward_iterator& operator++();
----------------
ldionne wrote:
> Quuxplusone wrote:
> > This seems like overkill. (Certainly `adl_forward_iterator` needn't be a template! And you see why inheriting from things-that-aren't-meant-for-inheritance is a bad idea.)
> > I recommend replacing this whole thing with two lines, which can go in their respective .pass.cpp files:
> > ```
> > static_assert(std::is_class_v<decltype(std::ranges::uninitialized_default_construct)>);
> > static_assert(std::is_class_v<decltype(std::ranges::uninitialized_default_construct_n)>);
> > ```
> > I recommend adding a bonus test case for their SFINAE, as well:
> > ```
> > struct NotDefaultCtable { NotDefaultCtable(int); };
> > static_assert(!std::is_invocable_v<decltype(std::ranges::uninitialized_default_construct), NotDefaultCtable*, NotDefaultCtable*>);
> > static_assert(!std::is_invocable_v<decltype(std::ranges::uninitialized_default_construct_n), NotDefaultCtable*, int>);
> > ```
> > [Using `static_assert(std::is_class_v<...>)`]
> 
> Interesting trick! It's true that if it's a class type, then ADL for sure won't trigger, so we'd have the same coverage. However, implementations are technically allowed to use functions (it would presumably be implemented with some sort of attribute) to inhibit ADL, so this would have to be `LIBCPP_STATIC_ASSERT` because `std::is_class_v` would be libc++ specific.
> 
> I don't have a strong opinion about this, but I agree this is pretty attractive from a simplicity perspective.
> implementations are technically allowed to use functions

Sure, //technically//, but until literally anyone does even a //proof of concept// patch to literally any compiler in the world to permit that, I think we can discount the possibility. ;)  (This is closely adjacent to my logic against `__function_like`, btw. Everyone knows that niebloids are objects-not-functions; there's no engineering benefit gained by pretending things aren't as they are, and meanwhile there //are// engineering costs.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115315