[libcxx-commits] [PATCH] D115626: [libc++][ranges] Implement `uninitialized_value_construct{, _n}` and `uninitialized_fill{, _n}`.

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__memory/voidify.h:25-29
+{
+  // Note: a C-style cast can cast away cv-qualifiers (it can be equivalent to
+  // doing a `static_cast` followed by a `const_cast`), thus achieving the
+  // same effect as the Standard mandates.
+  return (void*)_VSTD::addressof(__from);
----------------
ldionne wrote:
> var-const wrote:
> > ldionne wrote:
> > > Any reason why we don't literally copy-paste the definition from the Standard?
> > This is based on a previous (unfinished) discussion in [D115315](https://reviews.llvm.org/D115315). I think this form was advocated by @Quuxplusone. I didn't have a strong preference at the time of the discussion, but currently I'm leaning towards going with the Standard's definition.
> > 
> > My understanding is that the main advantage of using the C-style cast is its simplicity. However, it now seems to me that it's not so much simplicity as brevity, which isn't the same thing. The implementation from the Standard is, without doubt, clunky, however it a) explains exactly what's going on and b) draws attention to the fact that something unusual is going on. A C-style cast, however, can do one of several things, and remembering the rules and going through the exercise of applying them in this case is, I think, an extra burden for the reader; furthermore, given the C-style cast's brevity, the reader may be _unlikely_ to realize it's even an exercise worth going through because nothing really draws attention to it (other than "Why are we using a C-style cast?").
> > 
> > Finally, copy-pasting the Standard's definition is conceptually simpler. For the reader, there is no need to mentally verify that the two forms achieve exactly the same effect.
> > 
> > For these reasons, I'm currently leaning towards changing this to use the `static_cast` + `const_cast` combo. However, I would wait for @Quuxplusone's comments before making the change.
> > 
> I +1 going with the standard's definition (and that's what I did when I first introduced `__voidify` before we removed it).
I'm okay with reintroducing `__voidify` and copy-pasting from the Standard.
(//Personally// I still think there was nothing wrong with `(void*)`, but if you're waiting for my feedback here, I'm saying "do what Louis says.")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115626