Quuxplusone added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:377
+ "%0 is a reserved identifier">,
+ InGroup<ReservedIdentifier>, DefaultIgnore;
+
----------------
If you leave it like this, you //will// receive multiple bug reports per year
about how in some contexts the compiler diagnoses `_X` and `_x` equally, and in
other contexts the compiler diagnoses `_X` but fails to diagnose `_x`.
You should make two diagnostics: `-Wreserved-extern-identifier` and
`-Wreserved-pp-identifier` (or something along those lines), and their messages
should be carefully worded to explain //why// the warning is happening —
"identifier %0 is reserved for use at file scope," "identifier %0 is reserved
in all contexts," etc.
Btw, the reason some identifiers (like `_X`) are reserved in all contexts is so
that the implementation can define them as macros (e.g. header guards).
================
Comment at: clang/test/Sema/reserved-identifier.c:9
+int _foo() { return 0; } // expected-warning {{'_foo' is a reserved
identifier}}
+
+// This one is explicitly skipped by -Wreserved-identifier
----------------
aaron.ballman wrote:
> Can you add a test that we do not warn on an extern declaration? e.g.,
> ```
> extern char *_strdup(const char *);
> ```
> This is sometimes used (esp in older C code bases) to avoid having to include
> an entire header to scrape one declaration out of it, and there are popular
> libraries (like the MSVC CRT) that have APIs starting with a single
> underscore and lowercase letter.
>
> The idea here is: if it's an extern declaration, the identifier is "owned" by
> some other declaration and this is not likely something the user has control
> over.
Should that logic also apply to a forward declaration like `struct _foo;`?
Should it apply to `struct _foo { int i; };`? These are also things the user
might not have control over. (And they're equally things that the user
//could// pull out into a separate .h file surrounded by disabled-warning
pragmas, if they really wanted to.)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93095/new/
https://reviews.llvm.org/D93095
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
