[PATCH] D106577: [clang] Define __STDC_ISO_10646__

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 26 11:01:54 PDT 2021


rsmith added a comment.

In D106577#2904078 <https://reviews.llvm.org/D106577#2904078>, @aaron.ballman wrote:

> In D106577#2902091 <https://reviews.llvm.org/D106577#2902091>, @jyknight wrote:
>
>> In D106577#2901654 <https://reviews.llvm.org/D106577#2901654>, @rsmith wrote:
>>
>>> I think this patch as it stands would cause problems with libc implementations that put their `#define` somewhere other than than `stdc-predef.h` (eg, older versions of glibc that put it in `features.h` or other standard libraries that put it in `yvals.h`) due to the macro redefinition with a conflicting expansion. Such implementations are non-conforming but certainly exist. Likewise it could break the build for the same reason if we start implicitly including `stdc-predef.h` at some point. So I think that blindly predefining this macro will prove problematic in at least some circumstances.
>>
>> I don't understand what problem you expect to see here. We already suppress -Wmacro-redefined [which this falls under] for system headers -- doesn't that suffice?
>
> I'd like to hear more about this as well, FWIW. I think we get all the benefits you describe by defining the macro in the frontend and letting the macro suppression work if there's a conflicting definition from a system header.

I'd expect we will break at least things like libc++ tests that build with `-Wsystem-headers` in the case where libc defines the macro. But if the problem is limited to old libc and a rare use case, and can easily be worked around with a `-Wno` flag, that's probably fine.

One benefit we don't get with this approach is providing the right value for the macro (without paying the cost of always including `stdc-predefs.h`). AFAICS, the only possible use for the value of the macro is to detect libc support, so having Clang pick a specific value seems wrong to me. In some ways I'd be more comfortable with this patch if we defined the macro to `1` and documented that we think WG14 was wrong to ask for a version number.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106577



More information about the cfe-commits mailing list