rsmith added inline comments.
================
Comment at: clang/lib/Sema/Sema.cpp:2421-2428
+ // Perform a lookup at TUScope. If it succeeds, we're at global scope and a
+ // single '_' is enough to be reserved.
+ LookupResult IdentifierLookup(*this, II, SourceLocation(),
+ Sema::LookupAnyName,
+ Sema::ForExternalRedeclaration);
+ IdentifierLookup.suppressDiagnostics();
+ if (LookupName(IdentifierLookup, TUScope))
----------------
I don't understand why name lookup is involved here. Whether an identifier is
reserved doesn't depend on whether it's already been declared in the global
namespace or not. It's valid to declare `_foo` in some user-defined namespace
regardless of whether there's already a `_foo` in the global namespace, and
it's invalid to declare `_foo` in the global namespace regardless of whether
there's already a `_foo` in the global namespace.
If you're trying to detect whether the name is being introduced in the global
namespace scope, per C++ [lex.name]/3.2, you can't do that like this; you'll
need to look at the `DeclContext` of the declaration instead of just the
identifier.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:5898
+ if (D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_extern)
+ warnOnReservedIdentifier(New);
----------------
Why does the presence / absence of `extern` matter here?
================
Comment at: clang/lib/Sema/SemaDecl.cpp:13640
+ warnOnReservedIdentifier(New);
+
----------------
Is there somewhere more central you can do this, rather than repeating it once
for each kind of declaration? (Eg, `PushOnScopeChains`)
================
Comment at: clang/lib/Sema/SemaDecl.cpp:16296
} else {
+ if (TUK == TUK_Definition)
+ warnOnReservedIdentifier(New);
----------------
Why do we not diagnose the other possible `TagUseKind`s? `struct _foo;` and
`struct _foo *p;` both use reserved identifiers too.
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
