[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/include/__memory/ranges_uninitialized_algorithms.h:23
+#include <__ranges/dangling.h>
+#include <concepts>
+#include <type_traits>
----------------
Please include the granular `<__concepts/foo.h>` headers here [er, in a-z order above], not all of `<concepts>`.


================
Comment at: libcxx/include/__memory/ranges_uninitialized_algorithms.h:37
+
+struct __uninitialized_default_construct_fn final : private __function_like {
+
----------------
This should be as in the blog post too:
```
namespace __uninitialized_default_construct {
struct __fn {
};
} // namespace __uninitialized_default_construct
```
AIUI, this keeps the CPO's own type from ADL'ing into the `std::ranges` namespace; e.g. `foobar(std::ranges::uninitialized_default_construct)` should not consider `std::ranges::foobar` a candidate //even if std::ranges::foobar is not a CPO itself.//
Also, of course, consistency (Chesterton's Fence, the economist's hundred-dollar bill): if it were safe to omit the namespace, we'd certainly want to do it everywhere, not just here.


================
Comment at: libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.next/special_function.compile.pass.cpp:91
 
+// TODO(varconst): simply check that `next` is a variable and not a function.
 // When found by unqualified ([basic.lookup.unqual]) name lookup for the postfix-expression in a
----------------
Yay. :) IMHO a more helpful pattern for this comment would just be
`// TODO: make consistent with std/iterato...compile.pass.cpp`
and name one of the files that you consider to be correct. Then, even someone-not-you should be able to deduce exactly what this comment is asking them to do.


================
Comment at: libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.default/uninitialized_default_construct.pass.cpp:90
 
-void test_value_initialized()
-{
----------------
Btw, please commit this removal in a separate commit, and/or make sure there's something in your commit message about how we believe this was UB and that's why it's gone now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115315