[PATCH] D146986: Downgrade reserved module identifier error into a warning

Mon Mar 27 12:00:51 PDT 2023

aaron.ballman added a comment.

In D146986#4225121 <https://reviews.llvm.org/D146986#4225121>, @dblaikie wrote:

> From the discussion on the issue:
>
>> Do we want this loosening of the restriction to apply to *only* `std` and the same followed by a number, or to any reserved identifier used in a module? e.g.,
>>
>>   module std; // error today, but will become a warning
>>   module _Test; // error today, but do we want this to become a warning as well?
>>
>> my thinking is we probably want all of these to be warnings because it'd be hard to explain why `std` is reserved but with a warning while `_Test` is reserved but with an error.
>
> Yeah, I'd treat them equally - while we could subset the reserved names and allow implementations to only use a subset (while leaving the rest as an error for both implementations and consumers alike) that doesn't feel in keeping with the purpose of these names - to be usable by /someone/ and so necessary to allow them to be used.
>
> (hmm - there's some discussion in the description about the fact that this error was already suppressed in "system headers" - why was that suppression inadequate for system implementation modules? (& does that suppression for reserved names risk being over-broad, since every third party library installed on a system is generally considered a "system header", even if they aren't part of the implementation?))

We currently use line markers to "enter" a system header and that's quite fragile. I mentioned we could use `#pragma clang system_header`, but @ChuanqiXu  didn't think that was appropriate because these are not headers, they're modules, and we should have some separation between "system headers" and "system modules". (https://github.com/llvm/llvm-project/issues/61446#issuecomment-1473029776) As for being over-broad, it might be, but this is the approach we usually take (anything that's a "system header" is considered special and gets less diagnostics because the user isn't typically able to change the contents of the header file anyway).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146986