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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante 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)",
   }, {
----------------
Quuxplusone wrote:
> 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.
I had a quick look at it and I think @ldionne's proposal is better. I think it would be good to have a look at this script and improve the naming and also improve the documentation at https://libcxx.llvm.org/docs/DesignDocs/FeatureTestMacros.html.

I also noticed a `lit_markup` in the script, this seems quite useful to disable all tests using concepts when the compiler doesn't support it.

I'll have a closer look later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97904