[libc++] Fix PR35491 - std::array of zero-size doesn't work with non-default constructible types.

[EricWF]

Summary

This patch fixes llvm.org/PR35491 and LWG2157 (https://cplusplus.github.io/LWG/issue2157)

The fix attempts to maintain ABI compatibility by replacing the array with a instance of aligned_storage.

[EricWF]

Fix begin/end/data.

[lichray]

include/array

135

Just asking: no compiler is dumb enough to not inline this entirely trivial wrapper so that it's okay to not to propagate noexcept here to avoid try...capture...terminate codegen in its caller side?

[mclow.lists]

I'm wondering if it's not a better idea to have an explicit specialization for size == 0

template <class T>
class array<T, 0> {
// and so on.
};

[lichray]

BTW, this is https://cplusplus.github.io/LWG/issue2157 , so please update www accordingly, and also test {{}} cases.

[EricWF]

In D41223#957308, @mclow.lists wrote:

I'm wondering if it's not a better idea to have an explicit specialization for size == 0

template <class T>
class array<T, 0> {
// and so on.
};

I think that's probably more work than it's worth. I think we should change the primary template to support the bare minimum number of operations, and then let the rest of the operations blow up if users attempt to use them.

In D41223#957334, @lichray wrote:

BTW, this is https://cplusplus.github.io/LWG/issue2157 , so please update www accordingly,

The www pages will get updated when the issue is actually accepted, which it currently isn't.
There is no place to put that information now.

and also test {{}} cases.

I don't agree those cases are valid. That initialization syntax suggests there is something in the zero-sized array which is default constructible - or that users should be able to control construction of. I disagree with that.

include/array

135

_LIBCPP_INLINE_VISIBILITY should force inlining.

[EricWF]

In D41223#997264, @EricWF wrote:

and also test {{}} cases.

I don't agree those cases are valid. That initialization syntax suggests there is something in the zero-sized array which is default constructible - or that users should be able to control construction of. I disagree with that.

Nevermind, I see STL is insisting that case be handled. I'll add tests and then complain to him later.

[EricWF]

• Address review comments.
• Add initialization tests.

[EricWF]

This was reverted as it caused build failures. Re-opening with new version.

[EricWF]

• Update with new, hopefully working, version.

[EricWF]

Accepting new version.

[jtbandes]

The lack of _LIBCPP_CONSTEXPR_AFTER_CXX14 on the array<T, 0> specialization's begin(), end(), and other methods seems to be a bug: https://stackoverflow.com/questions/60462569/empty-stdarrayt-0-doesnt-have-constexpr-begin

[EricWF]

In D41223#1899394, @jtbandes wrote:

The lack of _LIBCPP_CONSTEXPR_AFTER_CXX14 on the array<T, 0> specialization's begin(), end(), and other methods seems to be a bug: https://stackoverflow.com/questions/60462569/empty-stdarrayt-0-doesnt-have-constexpr-begin

That bug is llvm.org/PR40124.

There are good reasons why it hasn't been fixed yet.

include/array

 #pragma GCC system_header
 #endif
 
 
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <class _Tp, size_t _Size>
+struct __array_traits {
+  typedef _Tp _StorageT[_Size];
+
+  _LIBCPP_INLINE_VISIBILITY
+  static _LIBCPP_CONSTEXPR_AFTER_CXX14 _Tp* __data(_StorageT& __store) {
+    return __store;
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  static _LIBCPP_CONSTEXPR_AFTER_CXX14 _Tp const* __data(const _StorageT& __store) {
+    return __store;
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  static void __swap(_StorageT& __lhs, _StorageT& __rhs) {
+    std::swap_ranges(__lhs, __lhs + _Size, __rhs);
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  static void __fill(_StorageT& __arr, _Tp const& __val) {
+    _VSTD::fill_n(__arr, _Size, __val);
+  }
+};
+
+template <class _Tp>
+struct __array_traits<_Tp, 0> {
+  typedef typename aligned_storage<sizeof(_Tp), alignment_of<_Tp>::value>::type _StorageT;
+
+  _LIBCPP_INLINE_VISIBILITY
+  static _Tp* __data(_StorageT& __store) {
+    _StorageT *__ptr = std::addressof(__store);
+    return reinterpret_cast<_Tp*>(__ptr);
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  static const _Tp* __data(const _StorageT& __store) {
+    const _StorageT *__ptr = std::addressof(__store);
+    return reinterpret_cast<const _Tp*>(__ptr);
+  }
+
+  _LIBCPP_INLINE_VISIBILITY
+  static void __swap(_StorageT&, _StorageT&) {}
+
+  _LIBCPP_INLINE_VISIBILITY
+  static void __fill(_StorageT&, _Tp const&) {
+  }
+};
+
+template <class _Tp, size_t _Size>
 struct _LIBCPP_TEMPLATE_VIS array
 {
     // types:
     typedef array __self;
     typedef _Tp                                   value_type;
     typedef value_type&                           reference;
     typedef const value_type&                     const_reference;
     typedef value_type*                           iterator;
     typedef const value_type*                     const_iterator;
     typedef value_type*                           pointer;
     typedef const value_type*                     const_pointer;
     typedef size_t                                size_type;
     typedef ptrdiff_t                             difference_type;
     typedef std::reverse_iterator<iterator>       reverse_iterator;
     typedef std::reverse_iterator<const_iterator> const_reverse_iterator;
 
-    value_type __elems_[_Size > 0 ? _Size : 1];
+    typedef __array_traits<_Tp, _Size> _Traits;
+    typename _Traits::_StorageT __elems_;
 
     // No explicit construct/copy/destroy for aggregate type
     _LIBCPP_INLINE_VISIBILITY void fill(const value_type& __u)
-        {_VSTD::fill_n(__elems_, _Size, __u);}
-    _LIBCPP_INLINE_VISIBILITY
-    void swap(array& __a) _NOEXCEPT_(_Size == 0 || __is_nothrow_swappable<_Tp>::value)
-        { __swap_dispatch((std::integral_constant<bool, _Size == 0>()), __a); }
-
-    _LIBCPP_INLINE_VISIBILITY
-    void __swap_dispatch(std::true_type, array&) {}
+        {_Traits::__fill(__elems_, __u);}
 
     _LIBCPP_INLINE_VISIBILITY
-    void __swap_dispatch(std::false_type, array& __a)
-        { _VSTD::swap_ranges(__elems_, __elems_ + _Size, __a.__elems_);}
+    void swap(array& __a) _NOEXCEPT_(_Size == 0 || __is_nothrow_swappable<_Tp>::value)
+        { _Traits::__swap(__elems_, __a.__elems_); }
 
     // iterators:
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
-    iterator begin() _NOEXCEPT {return iterator(__elems_);}
+    iterator begin() _NOEXCEPT {return iterator(data());}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
-    const_iterator begin() const _NOEXCEPT {return const_iterator(__elems_);}
+    const_iterator begin() const _NOEXCEPT {return const_iterator(data());}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
-    iterator end() _NOEXCEPT {return iterator(__elems_ + _Size);}
+    iterator end() _NOEXCEPT {return iterator(data() + _Size);}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
-    const_iterator end() const _NOEXCEPT {return const_iterator(__elems_ + _Size);}
+    const_iterator end() const _NOEXCEPT {return const_iterator(data() + _Size);}
 
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
     reverse_iterator rbegin() _NOEXCEPT {return reverse_iterator(end());}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
     const_reverse_iterator rbegin() const _NOEXCEPT {return const_reverse_iterator(end());}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
     reverse_iterator rend() _NOEXCEPT {return reverse_iterator(begin());}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
struct _LIBCPP_TEMPLATE_VIS array
     _LIBCPP_CONSTEXPR_AFTER_CXX11 const_reference at(size_type __n) const;
 
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14 reference front()             {return __elems_[0];}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11 const_reference front() const {return __elems_[0];}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14 reference back()              {return __elems_[_Size > 0 ? _Size-1 : 0];}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11 const_reference back() const  {return __elems_[_Size > 0 ? _Size-1 : 0];}
 
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
-    value_type* data() _NOEXCEPT {return __elems_;}
+    value_type* data() _NOEXCEPT {return _Traits::__data(__elems_);}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
-    const value_type* data() const _NOEXCEPT {return __elems_;}
+    const value_type* data() const _NOEXCEPT {return _Traits::__data(__elems_);}
 };
 
 template <class _Tp, size_t _Size>
 _LIBCPP_CONSTEXPR_AFTER_CXX14
 typename array<_Tp, _Size>::reference
 array<_Tp, _Size>::at(size_type __n)
 {
     if (__n >= _Size)

test/std/containers/sequences/array/array.cons/default.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 // <array>
 
 // array();
 
 #include <array>
 #include <cassert>
 
+// std::array is explicitly allowed to be initialized with A a = { init-list };.
+// Disable the missing braces warning for this reason.
+#include "disable_missing_braces_warning.h"
+
+struct NoDefault {
+  NoDefault(int) {}
+};
+
 int main()
 {
     {
         typedef double T;
         typedef std::array<T, 3> C;
         C c;
         assert(c.size() == 3);
     }
     {
         typedef double T;
         typedef std::array<T, 0> C;
         C c;
         assert(c.size() == 0);
     }
+    {
+      typedef std::array<NoDefault, 0> C;
+      C c;
+      assert(c.size() == 0);
+      C c1 = {};
+      assert(c1.size() == 0);
+      C c2 = {{}};
+      assert(c2.size() == 0);
+    }
 }

test/std/containers/sequences/array/array.data/data.pass.cpp

int main()
     }
     {
         typedef double T;
         typedef std::array<T, 0> C;
         C c = {};
         T* p = c.data();
         (void)p; // to placate scan-build
     }
+    {
+      struct NoDefault {
+        NoDefault(int) {}
+      };
+      typedef NoDefault T;
+      typedef std::array<T, 0> C;
+      C c = {};
+      T* p = c.data();
+      assert(p != nullptr);
+    }
 }

test/std/containers/sequences/array/array.data/data_const.pass.cpp

int main()
     }
     {
         typedef double T;
         typedef std::array<T, 0> C;
         const C c = {};
         const T* p = c.data();
         (void)p; // to placate scan-build
     }
+    {
+      struct NoDefault {
+        NoDefault(int) {}
+      };
+      typedef NoDefault T;
+      typedef std::array<T, 0> C;
+      const C c = {};
+      const T* p = c.data();
+      assert(p != nullptr);
+    }
 #if TEST_STD_VER > 14
     {
         typedef std::array<int, 5> C;
         constexpr C c1{0,1,2,3,4};
         constexpr const C c2{0,1,2,3,4};
 
         static_assert (  c1.data()  == &c1[0], "");
         static_assert ( *c1.data()  ==  c1[0], "");
         static_assert (  c2.data()  == &c2[0], "");
         static_assert ( *c2.data()  ==  c2[0], "");
     }
 #endif
 }

test/std/containers/sequences/array/begin.pass.cpp

int main()
         C c = {1, 2, 3.5};
         C::iterator i;
         i = c.begin();
         assert(*i == 1);
         assert(&*i == c.data());
         *i = 5.5;
         assert(c[0] == 5.5);
     }
+    {
+      struct NoDefault {
+        NoDefault(int) {}
+      };
+      typedef NoDefault T;
+      typedef std::array<T, 0> C;
+      C c = {};
+      assert(c.begin() == c.end());
+    }
 }