[libcxx-commits] [PATCH] D127953: [libc++] Mark standard-mandated includes as such

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

@jloser @Mordante Do you have suggestions for how such a test would look like? I don't think it's feasible to check for every symbol that should be provided for each header and checking that header X is included in Y sounds a bit fragile to me.



================
Comment at: libcxx/include/array:124
 #include <type_traits>
 #include <version>
 
----------------
Mordante wrote:
> Should we add `<version>` to the mandatory list too? It provides `__cpp_lib_array_constexpr` which this header must provide.
I don't think we want to add `<version>` because it provides more than by the standard required. We might want to only add the macros needed for each header in them, like libstdc++ and the MSVC STL do. (Although that's not very high on my priority list)


================
Comment at: libcxx/include/array:127
+// standard-mandated includes
+#include <__iterator/access.h>
+#include <__iterator/data.h>
----------------
Mordante wrote:
> Since these aren't obvious how do you feel about adding a comment like  `// [iterator.range]/1` to give a hint why it's needed. 
> 
> `<compare>` and `<initializer_list>` are obvious, so I don't feel to strong about these.
I think it's a good idea to say which section mandates them. Added comments for all headers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127953