[libcxx-commits] [PATCH] D80790: [libc++] Remove redundant empty specialization in std::array

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Fri May 29 12:00:31 PDT 2020


ldionne marked an inline comment as done.
ldionne added a comment.

In D80790#2063595 <https://reviews.llvm.org/D80790#2063595>, @zoecarver wrote:

> > I'm not sure what huge benefit that would yield.
>
> We don't have to, that's just a suggestion. I think it would be easier to review and if there's a bug in one part of the implementation, we have to revert less code (and it's easier to track down).
>
>  ---
>
> I think the direction here will depend on D80821 <https://reviews.llvm.org/D80821>.
>
> > We can't do that because array has to be an aggregate. It has to contain the T __elems[N] array as its sole member.
>
> I think we can inherit from `__array_storage` as a public base. The reason I think it's important to have the `data` logic factored out into another object is that if we do what I suggest in D80821 <https://reviews.llvm.org/D80821> we're going to have four overloads which will be more difficult to maintain if they're all in one class. If we don't do what I suggest in D80821 <https://reviews.llvm.org/D80821>, then I'm fine with the implementation in this patch.


Not prior to C++17. Aggregates can't have base classes in C++11 and C++14.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80790





More information about the libcxx-commits mailing list