[libcxx-commits] [PATCH] D105597: [libcxx][nfc] Remove <variant>'s dependence on <array>.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante accepted this revision as: Mordante.
Mordante added a comment.

LGTM!



================
Comment at: libcxx/include/variant:242
+struct __light_array {
+  _Tp __buff_[_Size] = {0};
+
----------------
Mordante wrote:
> Quuxplusone wrote:
> > Mordante wrote:
> > > Quuxplusone wrote:
> > > > What is `_Tp` here? Are we sure `0` is convertible to `_Tp`? (Is `_Tp` always just `size_t`? That'd be super convenient. :))
> > > > Are we sure `_Size` is never zero?
> > > Since the variable is public please drop the trailing underscore. Small nit, I would prefer `__buf` instead of `__buff`, the latter looks weird to me.
> > @mordante: I'd strongly prefer to keep the trailing underscore on data members, to distinguish e.g. the data member `__buf_` from the parameter `__buf`. (And I don't think we can make it `private` because then this type wouldn't get aggregate initialization.)
> I had a look and it seems `std::array` also uses a trailing underscore for its public member, so in that case I no longer have any objections against the trailing underscore. Still prefer `__buf_` over `__buff_`.
> 
> I agree the member should be public, it was not my intention to imply it should become private.
@zoecarver It seem I typed my previous message while you posted a new version. So that issue has been addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105597