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

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Dec 8 12:16:44 PST 2021


Quuxplusone added inline comments.


================
Comment at: libcxx/include/__memory/uninitialized_algorithms.h:132
     for (; __idx != __last; ++__idx)
-        ::new ((void*)_VSTD::addressof(*__idx)) _Vt;
+        ::new ((void*)_VSTD::addressof(*__idx)) _ValueType;
 #ifndef _LIBCPP_NO_EXCEPTIONS
----------------
ldionne wrote:
> Quuxplusone wrote:
> > var-const wrote:
> > > Note: this cast is simpler than the [`voidify`](http://eel.is/c++draft/specialized.algorithms#general-4) function that the Standard mandates. Should I fix this, or is there a reason why a simple cast to `void*` is sufficient here?
> > I can't think of any reason a simple cast to `void*` would be //insufficient//. I'd leave it simple until-and-unless someone comes up with a positive reason and gives us a regression test case. Otherwise, you'll complexify the code and someone six months from now will ask why it can't be simpler.
> When @Quuxplusone originally removed `__voidify` in D93153, I was fine with it. I just did some additional research and I suspect it might have been wrong: https://githubmemory.com/repo/microsoft/STL/issues/1780
> 
> I think we should figure out what the situation is and either confirm that we're conforming, or fix all the places where we use `(void*)x` but the standard says `__voidify(x)`.
(Argh, I keep forgetting to reply to things on the first go-round. Sorry.)
IIUC, STL's comment there indicates that we should add a test case for const iterators, e.g.
```
auto cit = forward_iterator<const Counted*>(as_array);
auto r = std::ranges::subrange(cit, sentinel_wrapper<forward_iterator<const Counted*>>(cit + 2));
std::ranges::uninitialized_default_construct(r.begin(), r.end());
assert(Counted::current_objects == 2);
assert(Counted::total_objects == 2);
```
That's missing test coverage today, but AFAIK it's not an argument against the simple cast to `(void*)`. C-style casts are allowed to cast away constness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115315



More information about the libcxx-commits mailing list