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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 26 11:36:20 PDT 2022


aaron.ballman added a comment.

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

> In D120244#3540598 <https://reviews.llvm.org/D120244#3540598>, @aaron.ballman wrote:
>
>> The system header including a header that's explicitly deprecated is dubious practice, but that does still require some amount of timing coordination.
>
> I agree, and I'll be filing bug reports against system headers within my control that use `<stdbool.h>`. That is unfortunately not the majority of headers, though.

Thanks!

>> 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?
>
> I fully agree -- we need a way to deprecate headers (just like we do for classes and functions). My intent is really not to impair your efforts at doing that -- I'm just trying to point out the unfortunate tradeoff we are making. Regarding `Foundation.h` -- I am using this as an example, but it's really just the tip of the iceberg. We can probably get this one fixed within a reasonable time frame, however that won't change the fundamental issue.

Agreed, the issue is that `#warning` is effectively useless in system headers.  :-(

>> 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`.
>
> Yes, if that is feasible, I think something like that is exactly what we would need. If we had a tool like this, by the way, I would be very keen on deprecating some libc++ headers that we keep around like `<ciso646>` & friends.

I'll explore just how possible this is. There's all sorts of neat problems with it, like header guards/`#pragma once` meaning the header file's contents aren't scanned on a second inclusion (so we need to keep a list of already-included files that are deprecated), and truly awful things like:

  // Foo.h, which has no header guard because it's like assert.h
  #if WHATEVER
  ...
  #else
  #pragma clang deprecate_header
  #endif
  
  // main.c
  #define WHATEVER
  #include "Foo.h"
  #undef WHATEVER
  #include "Foo.h"
  #define WHATEVER
  #include "Foo.h"
  #endif

and so on.

> So, in summary, my opinion is that we should wait until we have the appropriate technology (something like proposed above) before deprecating these headers. And once we do, then I'll be 100% supportive of efforts in that direction. But I'm mostly relaying user feedback here, I don't own the Clang headers so I must defer to your judgement.

The feedback from your users (and you) is very much appreciated; this is exactly the kind of feedback I hoped to get by doing those changes early in the release (so I still have time to address issues like this before it's too late). So thank you for bringing this up and the discussion around it!

I'll roll back the `#warning` use and report back when that's done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120244



More information about the cfe-commits mailing list