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

[Louis Dionne via libcxx-commits]

ldionne 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.

> 
> 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.

> 
> OTOH, it seems suspicious that we're setting these in the first place. If the conditions in `__config` lead to libc++ defining `#define _LIBCPP_HAS_FOO 1` but we have `-D_LIBCPP_HAS_FOO=0` as a build flag, that would break due to macro redefinition w/ a different value, right? Are there any conditions here where it would make sense for `__config` to only define it if not already defined?

Exactly, right. So libc++ has a notion of "user configurable settings", but not all the `_LIBCPP_HAS_NO_FOO` fall into that category. Unfortunately, whether something is intended to be user-configurable or not is usually not well documented and it's mostly something that the developers know.

In the general case, users are not expected to define any `_LIBCPP` macro unless it's documented as a public configurable thing. In practice, people probably often define macros in ways we haven't thought about.

> 
> 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.

IMO this patch can be a first step towards formalizing what is and isn't a user-configurable setting. We could potentially use a different naming convention to make it more obvious what is configurable. Or we could potentially do something like

```
#ifdef _LIBCPP_HAS_EXCEPTIONS
#  error "_LIBCPP_HAS_EXCEPTIONS is not supposed to be user-configured, it's supposed to be inferred by libc++"
#endif

#if __has_feature(__cxx_exceptions)
#  define _LIBCPP_HAS_EXCEPTIONS 1
#else
#  define _LIBCPP_HAS_EXCEPTIONS 0
#endif
```

That would make it clear what we intend for users to set and what we don't. But any such thing should be left for a different patch.

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