[libcxx-commits] [PATCH] D60666: Fix some constexpr members in array<T, 0>

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: include/array:315
     // element access:
-    _LIBCPP_INLINE_VISIBILITY
+    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
     reference operator[](size_type) _NOEXCEPT {
----------------
This can't be marked `constexpr` because it never produces a constant expression.


================
Comment at: include/array:327
 
-    _LIBCPP_INLINE_VISIBILITY
+    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
     reference at(size_type) {
----------------
Ditto, and several times below.


================
Comment at: include/array:364
+    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
+    value_type* data() _NOEXCEPT {return &__w.__t;}
+    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
----------------
You need to do something different based on whether you're in C++17 or not. Make sure you run the tests with multiple standards!


================
Comment at: test/std/containers/sequences/array/size_and_alignment.pass.cpp:24
+template <class A>
+inline constexpr bool has_consexpr_members(A const& arr) {
+    return  arr.begin()                         &&
----------------
This test is always enabled (even in C++03 mode), so I don't think you can use `constexpr` here. Also, `inline` isn't useful here.

Finally, you only need this test in C++17, so you can use generalized `constexpr` and not do only one awkwardly long expression.


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

https://reviews.llvm.org/D60666