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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne marked 2 inline comments as done.
ldionne added a comment.

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

> What if we did this in three patches?
>
> 1. Update everything to use `data()` instead of `__elems_`.
> 2. Use `__array_storage` in both implementations.
> 3. Remove the `array<T, 0>` specialization.


I'm not sure what huge benefit that would yield.

> It also might be good to move `data()` into `__array_storage` and hold an instance of `__array_storage` itself (the element storage would exist in `__array_storage`). That would make it easier to have many specialized overloads, i.e. one for `N = 0` where `T` is trivial, where `N = 0` and `T` is not trivial, and where `N != 0`. Then the `array` implementation only has to worry about implementing itself in terms of `data()`.

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.



================
Comment at: libcxx/include/array:128
 
+namespace __detail {
+    template <typename _Tp, size_t _Size>
----------------
zoecarver wrote:
> Do we use a `__detail` namespace anywhere else? I don't have an issue with it but, it seems a bit out of place. 
True, I can remove it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80790