[libcxx-commits] [PATCH] D80452: [libc++] Complete overhaul of constexpr support in std::array

[Michael Schellenberger Costa via Phabricator via libcxx-commits]

miscco added a comment.

Thanks a lot for improving the tests. I am not yet comfortable with libc++ tests so I have some general questions inline



================
Comment at: libcxx/test/std/containers/sequences/array/array.cons/deduct.pass.cpp:33
 
-int main(int, char**)
+template <typename Dummy = void>
+constexpr bool tests()
----------------
being new to the party, what is the reason for the template?


================
Comment at: libcxx/test/std/containers/sequences/array/array.cons/implicit_copy.pass.cpp:39
+    {
+        typedef double T;
+        typedef std::array<T, 3> C;
----------------
Would it make sense to consistently drop all those typedefs?

If not what is their purpose?


================
Comment at: libcxx/test/std/containers/sequences/array/array.cons/initializer_list.pass.cpp:27
         typedef std::array<T, 3> C;
-        C c = {1, 2, 3.5};
+        C const c = {1, 2, 3.5};
         assert(c.size() == 3);
----------------
preexisting: Are those conversions from into double intentional?


================
Comment at: libcxx/test/std/containers/sequences/array/array.cons/initializer_list.pass.cpp:43
         typedef std::array<T, 3> C;
-        C c = {1};
+        C const c = {1};
         assert(c.size() == 3.0);
----------------
Same here int -> double conversion


================
Comment at: libcxx/test/std/containers/sequences/array/array.data/data.pass.cpp:37
+template <typename Dummy = void>
+TEST_CONSTEXPR_CXX14 bool tests()
 {
----------------
This is `TEST_CONSTEXPR_CXX14` but the constexpr test is `#if TEST_STD_VER >= 17`

Could we define a macro `TEST_CONSTEXPR_CXX14`?

Notably this should not compile for C++14 as data is not constexpr.

Occurs at other places too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80452