[PATCH] D41148: [libcxx] implement <experimental/simd> declarations based on P0214R7.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.


================
Comment at: libcxx/include/experimental/simd:1
+#ifndef _LIBCPP_EXPERIMENTAL_SIMD
+#define _LIBCPP_EXPERIMENTAL_SIMD
----------------
Please add the license header similar to `experimental/filesystem`.


================
Comment at: libcxx/include/experimental/simd:581
+*/
+
+#include <array>
----------------
The first include should be `<experimental/__config>`


================
Comment at: libcxx/include/experimental/simd:590
+
+#if _LIBCPP_STD_VER < 11
+#error "Pre-C++11 is not supported"
----------------
(1) `_LIBCPP_STD_VER` is never less than 11. It's an odd libc++ quirk.
(2) We don't normally generate explicit errors when headers are used incorrectly.



================
Comment at: libcxx/include/experimental/simd:594
+
+#warning "<experimental/simd> is under construction and not for practical use for now."
+
----------------
I would just remove this. The `experimental` part of the name should say enough.

Also we normally wait until an implementation is mostly complete before landing it. 


================
Comment at: libcxx/include/experimental/simd:596
+
+namespace std {
+namespace experimental {
----------------
Please define macros to open and close the namespace similar to `_LIBCPP_BEGIN_NAMESPACE_EXPERIMENTAL_FILESYSTEM` in `<experimental/__config>`


================
Comment at: libcxx/test/std/experimental/simd/traits.pass.cpp:15
+// [simd.traits]
+// template <class T> struct is_abi_tag;
+// template <class T> inline constexpr bool is_abi_tag_v = is_abi_tag<T>::value;
----------------
Please create a directory called `simd/traits/` and then put each trait in a separate test file under that directory.
The `foo_v` tests should be in the same file as the `foo` tests.




https://reviews.llvm.org/D41148