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

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added a comment.

Thanks for all the test improvements @ldionne!

---

What's the point of all the `Dummy` templates?



================
Comment at: libcxx/include/array:352
+    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
+    value_type* data() _NOEXCEPT {return nullptr;}
+    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
----------------
This will probably break some people's code. I would much prefer not returning `nullptr` here. What's the downside of using something similar to D60666's implementation of `data()`?


================
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()
----------------
miscco wrote:
> being new to the party, what is the reason for the template?
I don't think the template is needed.


================
Comment at: libcxx/test/std/containers/sequences/array/array.data/data.pass.cpp:53
         T* p = c.data();
-        LIBCPP_ASSERT(p != nullptr);
+        (void)p;
     }
----------------
I don't see the point of this test anymore (lines 48-54). 


================
Comment at: libcxx/test/std/containers/sequences/array/array.data/data_const.pass.cpp:60
+        const T* p = c.data();
+        (void)p;
     }
----------------
Same as above. 


================
Comment at: libcxx/test/std/containers/sequences/array/array.data/data_const.pass.cpp:95
+        const T* p = c.data();
+        std::uintptr_t pint = reinterpret_cast<std::uintptr_t>(p);
+        assert(pint % TEST_ALIGNOF(T) == 0);
----------------
`pint` will always be zero, no? I think it would make more sense to test this with non-null data (`std::array<T, 1>` maybe). 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80452