[libcxx-commits] [PATCH] D97904: [libcxx] updates the feature-test macro generator

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/utils/generate_feature_test_macro_components.py:424-425
     "headers": ["numbers"],
-    "depends": "defined(__cpp_concepts) && __cpp_concepts >= 201907L",
-    "internal_depends": "defined(__cpp_concepts) && __cpp_concepts >= 201907L",
+    "depends": "!defined(_LIBCPP_HAS_NO_CONCEPTS)",
+    "internal_depends": "!defined(_LIBCPP_HAS_NO_CONCEPTS)",
   }, {
----------------
ldionne wrote:
> Here, I think you mean:
> 
> ```
> "depends": "defined(__cpp_concepts) && __cpp_concepts >= 201907L",
> "internal_depends": "!defined(_LIBCPP_HAS_NO_CONCEPTS)",
> ```
> 
> The way I understand it, `internal_depends` is for macro dependencies for libc++ itself, and `depends` is for macro dependencies of the test suite.
I agree with your understanding; my source-diving suggested to me that these fields should really be named `depends->libcxx_test_depends` and `internal_depends->version_header_depends` respectively. I'd be willing to submit that mechanical patch, after this one lands in some form.

No opinion on whether `<version>` should be testing `_LIBCPP_HAS_NO_CONCEPTS` or `__cpp_lib_concepts` directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97904