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

[via libcxx-commits]

EricWF wrote:

> > I have a preference for the existing mechanisms. They've not caused my any trouble in my time on the project.
> > I think the existing mechanisms encourage/enforce the use of negative feature test macro names (ex. `_LIBCPP_HAS_NO_EXCEPTIONS`). I believe this naming scheme helps avoid misspelling/misconfiguration bugs, since a misspelled guard will almost certainly cause a compilation failure. I worry the proposed approach isn't as robust against this.
> 
> I'm not sure how the current mechanism is more robust. A misspelling might not be caught other than by testing the actual code (eg. `#ifdef _LIBCPP_HAS_NO_EXCEPTION` misspelled). 

Yes, like with `-Wundef` you need to compile the code.

That  said, lets say had

```c++
#ifndef _LIBCPP_HAS_NO_EXXXXCEPTIONS
throw 42;
#endif
```

This gets caught because the code fails to compile. If the code successfully compiles, that means there's some other bugs going on that needs fixing. So it catches both misspellings and misconfigurations.

Note that the negative naming of the macro matters here. It's And is important even with the new approach.

With this change, we open up a bunch of new failure modes.

I'm more ok with changing to 1 or 0 than I am removing the negative feature test names. Did the `<__availability>` bugs you referenced use negative feature test names? Those are important, and I would consider blocking this change if it meant using positive feature tests.

Let me know if I need to do a better job of explaining why positive names are less desirable than negative ones.

> As I said in the description, a very similar change in `<__availability>` caught real bugs.

Would you mind referencing the change? 

> 
> > This would change very long standing existing practice, which according to my recollection hasn't caused any major problems. Generally the macros are specified in the negative form (ex. _LIBCPP_HAS_NO_EXCEPTIONS), so they're only defined when a feature isn't available.
> > Because the current practice is so long standing, I would like this change to come with a clang-tidy that looked for misapplication of `#ifdef` to libc++ macros.
> 
> While the practice is long-standing, the names of the individual macros changed before ([cf65d27](https://github.com/llvm/llvm-project/commit/cf65d275ac048dfb781c7302c6c82c3500184eb1), b22aa3d, 5c40c99, 66a562d), without any major fallout I'm aware of. So I don't think a clang-tidy check pulls its weight here. A general check for internal macros might be nice, but is definitely out of scope for this change.

You're renaming the macros because macro usage is bug prone. I'm don't agree with the claim, and I don't think it's out of scope, given that this change is about catching macro bugs and is otherwise NFC. 

Let me know how you would like to proceed.


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