[libcxx-commits] [PATCH] D99615: [libc++] Improve generate_feature_test_macro_components.py.
Mark de Wever via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Sun Apr 4 10:58:32 PDT 2021
Mordante marked an inline comment as done.
Mordante added inline comments.
================
Comment at: libcxx/utils/generate_feature_test_macro_components.py:52
+# * `AVAILABILITY_DISABLE` macros defined in <__availability>.
+# It can't depend on <__config> since the test suite should
+# be portable. Using libc++ specific features will cause unit
----------------
curdeius wrote:
> This phrase is repeated just before the bullet list.
Thanks, fixed.
================
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:
> 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.
I like your text, will change `CMake, or` to `CMake.`
As suggested I'll then land this patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99615/new/
https://reviews.llvm.org/D99615
More information about the libcxx-commits
mailing list