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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

Thanks for working on this!

In D127953#3588836 <https://reviews.llvm.org/D127953#3588836>, @jloser wrote:

> This seems reasonable to me. Do you think it's worth adding a test for this behavior to avoid regression in the future, or do you think it's overkill?

I would like that. Maybe we can make automatically generated tests for some parts, like we already do for the feature-test macros.



================
Comment at: libcxx/include/array:124
 #include <type_traits>
 #include <version>
 
----------------
Should we add `<version>` to the mandatory list too? It provides `__cpp_lib_array_constexpr` which this header must provide.


================
Comment at: libcxx/include/array:127
+// standard-mandated includes
+#include <__iterator/access.h>
+#include <__iterator/data.h>
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127953