[PATCH] D120244: [clang][sema] Enable first-class bool support for C2x

Thu May 26 10:27:41 PDT 2022

aaron.ballman added a comment.

In D120244#3539852 <https://reviews.llvm.org/D120244#3539852>, @ldionne wrote:

> In D120244#3538660 <https://reviews.llvm.org/D120244#3538660>, @aaron.ballman wrote:
>
>> In D120244#3538380 <https://reviews.llvm.org/D120244#3538380>, @ldionne wrote:
>>
>>> We've started having several internal user complaints because their system headers are suddenly triggering a warning for using `<stdbool.h>`. This seems to be caused by the fact that `#warning` is surfaced to users even when the `#warning` is present in a system header (which makes sense, because otherwise there would be no way to issue a `#warning` from a system header). This ends up causing problems because users have no way to suppress the warning in the system headers they use without also disabling deprecation warnings in their own code. Is this intended? Instead, it seems to me like what we'd want is some way to mark the header as deprecated such that Clang will not flag uses of `<stdbool.h>` from within system headers, but will flag them from user code. This would be consistent with how the deprecated attributes work for classes and functions.
>>>
>>> Thoughts?
>>
>> This is what `_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS` is for. We documented it here: https://clang.llvm.org/docs/UsersManual.html#controlling-deprecation-diagnostics-in-clang-provided-c-runtime-headers but the basic idea is, if that's defined before including the header, we don't issue those warnings. Does that suffice?
>
> Well, the problem is that a user that is using some system header like `Foundation.h` (which includes `<stdbool.h>`) will be forced to define `_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS`, which has two drawbacks:
>
> 1. It's annoying to them because they never used `<stdbool.h>` in their own code
> 2. It turns off deprecation warnings in their own code as well

The system header including a header that's explicitly deprecated is dubious practice, but that does still require some amount of timing coordination.

> In other words, the fact that this `#warning` gets out of system headers means that it's going to "spam" users who have no control over what their system headers are doing. So while the long-long term fix is for all system headers (like `Foundation.h`) to be C2x friendly and not include `<stdbool.h>` in newer standard modes, the current migration path involves a lot of spammy warnings for end users in the meantime. Since such a migration may or may not happen completely everywhere, it also means that this is probably not a "bite the bullet for 3 months" situation.

Okay, that's a fair problem. However, the flip side is: why should C2x users not be told about deprecated functionality when they include the header directly?

> FWIW, although libc++ is fairly aggressive on marking functions and classes as deprecated, we do not try to warn for *headers* that are deprecated, because there is no satisfying way of doing so right now. Personally, my opinion is that unless we have a way to mark headers as deprecated in a way that the compiler can understand the semantics and have it behave roughly like a "deprecated attribute but on a header" (which means that this warning would behave just like all other compiler warnings w.r.t. system headers), it is not a great idea to do this. I'm very keen on deprecating stuff, however I think this `#warning` is not the right tool for it, because of the downsides I mentioned.

So I take it https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/stdnoreturn.h is also an issue?

My feeling is: system headers that spam warnings are a bigger concern than not getting a deprecation warning, but we want that deprecation warning eventually because we need *some* way to deprecate headers as a whole. Not issuing deprecation warnings means standards bodies can't reasonably rely on users having been warned, so not diagnosing means "let's stick the *entire ecosystem* with this header file forever". So I'm thinking of walking back the `#warning` for the headers but with some sort of timeline as to when we can add the diagnostics back to the headers. Do you have an idea of what would be reasonable for Foundation.h?

We could maybe extend `#pragma clang deprecate` to deprecate inclusion of the current file so that it acts sort of like a `[[deprecated]]` attribute that triggers on inclusion using typical diagnostics instead of `#warning`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120244