[PATCH] D22584: constexpr array support C++1z (P0031)

[Eric Fiselier via cfe-commits]

EricWF added a comment.

For the most part this LGTM. Thanks for putting in all this work. I've noted some places where the test coverage could be improved.


================
Comment at: include/array:156
@@ -155,3 +155,3 @@
     _LIBCPP_INLINE_VISIBILITY
-    iterator begin() _NOEXCEPT {return iterator(__elems_);}
+    _LIBCPP_CONSTEXPR_AFTER_CXX14 iterator begin() _NOEXCEPT {return iterator(__elems_);}
     _LIBCPP_INLINE_VISIBILITY
----------------
This is untested.

================
Comment at: include/array:160
@@ -159,3 +159,3 @@
     _LIBCPP_INLINE_VISIBILITY
-    iterator end() _NOEXCEPT {return iterator(__elems_ + _Size);}
+    _LIBCPP_CONSTEXPR_AFTER_CXX14 iterator end() _NOEXCEPT {return iterator(__elems_ + _Size);}
     _LIBCPP_INLINE_VISIBILITY
----------------
This is untested.

================
Comment at: include/array:165
@@ -164,3 +164,3 @@
     _LIBCPP_INLINE_VISIBILITY
-    reverse_iterator rbegin() _NOEXCEPT {return reverse_iterator(end());}
+    _LIBCPP_CONSTEXPR_AFTER_CXX14 reverse_iterator rbegin() _NOEXCEPT {return reverse_iterator(end());}
     _LIBCPP_INLINE_VISIBILITY
----------------
Untested.

================
Comment at: include/array:169
@@ -168,3 +168,3 @@
     _LIBCPP_INLINE_VISIBILITY
-    reverse_iterator rend() _NOEXCEPT {return reverse_iterator(begin());}
+    _LIBCPP_CONSTEXPR_AFTER_CXX14 reverse_iterator rend() _NOEXCEPT {return reverse_iterator(begin());}
     _LIBCPP_INLINE_VISIBILITY
----------------
Untested.

================
Comment at: include/array:202
@@ -201,3 +201,3 @@
     _LIBCPP_INLINE_VISIBILITY
-    value_type* data() _NOEXCEPT {return __elems_;}
+    _LIBCPP_CONSTEXPR_AFTER_CXX14 value_type* data() _NOEXCEPT {return __elems_;}
     _LIBCPP_INLINE_VISIBILITY
----------------
Untested.

================
Comment at: include/iterator:527
@@ -526,2 +526,3 @@
 inline _LIBCPP_INLINE_VISIBILITY
+_LIBCPP_CONSTEXPR_AFTER_CXX14
 void __advance(_InputIter& __i,
----------------
Untested.

================
Comment at: include/iterator:537
@@ -535,2 +536,3 @@
 inline _LIBCPP_INLINE_VISIBILITY
+_LIBCPP_CONSTEXPR_AFTER_CXX14
 void __advance(_BiDirIter& __i,
----------------
Untested.

================
Comment at: include/iterator:569
@@ -564,2 +568,3 @@
 inline _LIBCPP_INLINE_VISIBILITY
+_LIBCPP_CONSTEXPR_AFTER_CXX14
 typename iterator_traits<_InputIter>::difference_type
----------------
Untested.

================
Comment at: include/iterator:1158
@@ -1131,2 +1157,3 @@
 inline _LIBCPP_INLINE_VISIBILITY
+_LIBCPP_CONSTEXPR_AFTER_CXX14 
 typename move_iterator<_Iter1>::difference_type
----------------
This is a C++03 only branch, but I guess it's more consistent to apply the macro.

================
Comment at: test/std/containers/sequences/array/indexing.pass.cpp:41
@@ -26,1 +40,3 @@
+}
+
 int main()
----------------
Missing conditional directive terminator.

================
Comment at: test/std/containers/sequences/array/indexing.pass.cpp:84
@@ +83,3 @@
+    {
+        constexpr C c = constexpr_array_usage();
+        static_assert (c[0] == 0);
----------------
C doesn't name a type.

================
Comment at: test/std/containers/sequences/array/iterators.pass.cpp:112
@@ +111,3 @@
+
+#if _LIBCPP_STD_VER > 14
+    { // P0031 testing conextpr versions of iterators
----------------
Include `test_macros.h` and use `TEST_STD_VER` please.


https://reviews.llvm.org/D22584