[libcxx-commits] [PATCH] D99615: [libc++] Improve generate_feature_test_macro_components.py.

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

Quuxplusone accepted this revision.
Quuxplusone added inline comments.


================
Comment at: libcxx/utils/generate_feature_test_macro_components.py:56-59
+#                   It's possible to use the `AVAILABILITY_DISABLE` macros of
+#                   <__availability> since it defines libc++ specific macros,
+#                   that aren't defined by other C++ standard library
+#                   implementations.
----------------
Quuxplusone wrote:
> Wonky English grammar here; also, this logic applies equally well to //any// libc++ header (that libc++ headers define things that aren't defined by other library implementations).
> I think you should add a TODO and come back to this; i.e., take my suggested fix verbatim.
> These macros were all introduced in @ldionne's D94983, btw.
> 
Ah, D94983 enlightens me that the `FTM` in these macro names stands specifically for "feature-test macro"!
So I'd write it like this:
```
# test_suite_guard  An optional string field. When this field is provided,
#                   `libcxx_guard` must also be provided. This field is used
#                   only to generate the unit tests for the feature-test macros.
#                   It can't depend on macros defined in <__config> because the
#                   `test/std/` parts of the test suite are intended to be
#                   portable to any C++ standard library implementation, not
#                   just libc++. It may depend on
#                    * macros defined by the compiler itself,
#                    * macros generated by CMake, or
#                   In some cases we add `&& !defined(_LIBCPP_AVAILABILITY_DISABLE_FTM_...)`
#                   in order to make libc++ pass the tests on OSX; see D94983.
```

Anyway, I still say ship it and handle the cleanup post-commit.

Notice that you've got two copies of `libcxx_guard` now; delete the second copy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99615