[libcxx-commits] [libcxx] [libc++][RFC] Always define internal feature test macros (PR #89178)

Jordan Rupprecht via libcxx-commits libcxx-commits at lists.llvm.org
Thu Apr 18 12:01:24 PDT 2024


rupprecht wrote:

> > What kinds of adjustments downstream do you expect this will require?
> 
> Since this touches a lot of widely used macros, any downstream-only code that is conditionalized on exceptions would likely use `_LIBCPP_HAS_NO_EXCEPTIONS` and would need to change. That's just an example, but you get the idea.

Oh, I was only looking at build configurations, I completely forgot about that. I see quite a bit more when I expand the scope for things like that, e.g. https://github.com/abseil/abseil-cpp/blob/192e959b16809f751d565b53a949b21129d904fb/absl/hash/internal/hash.h#L73

Probably still tractable, but not as trivial as I thought.

It may help to temporarily leave in the old definitions for downstream use, even if libc++ itself does not use it, i.e.

```
#if __has_feature(__cxx_exceptions)
#  define _LIBCPP_HAS_EXCEPTIONS 1
#else
#  define _LIBCPP_HAS_EXCEPTIONS 0
#  ifdef _LIBCPP_DEFINE_DEPRECATED_FEATURE_TEST_MACROS  // <-- remove after an LLVM release or whatever
#    define _LIBCPP_HAS_NO_EXCEPTIONS
#  endif
#endif
```

(we may just need this for some popular subset of macros)

> 
> > Maybe it's just something historical, but I see some `-D_LIBCPP_HAS_NO_<x>` usage. Does this PR mean we should use `-D_LIBCPP_HAS_NO_FOO -D_LIBCPP_HAS_FOO=0` as a way to be compatible before & after this change?
> 
> If you can share which `FOO`s you found these usages for, that could also be enlightening.

_LIBCPP_HAS_NO_FILESYSTEM_LIBRARY
_LIBCPP_HAS_NO_INT128
_LIBCPP_HAS_NO_LIBRARY_ALIGNED_ALLOCATION
_LIBCPP_HAS_NO_LOCALIZATION
_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER
_LIBCPP_HAS_NO_STDIN
_LIBCPP_HAS_NO_STDOUT
_LIBCPP_HAS_NO_THREADS
_LIBCPP_HAS_NO_TIME_ZONE_DATABASE
_LIBCPP_HAS_NO_UNICODE
_LIBCPP_HAS_NO_VENDOR_AVAILABILITY_ANNOTATIONS

> > One common option I see being set is `_LIBCPP_HAS_NO_THREADS`, but that isn't being changed here. Did you miss that one, or is intentional that only a subset of the `HAS_NO` vars are being changed?
> 
> It looks like an oversight on @philnik777 's part. It also seems incorrect for users to be setting that, since this should be done via `__config_site` at configuration-time of the library.

It looks like all the usage is in build macros to generate a `__config_site` file.

https://github.com/llvm/llvm-project/pull/89178


More information about the libcxx-commits mailing list