serge-sans-paille marked an inline comment as done.
serge-sans-paille 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))
----------------
rsmith wrote:
> 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.
> If you're trying to detect whether the name is being introduced in the global
> namespace scope,
That's indeed the goal
> you'll need to look at the DeclContext of the declaration instead of just the
> identifier.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:13640
+ warnOnReservedIdentifier(New);
+
----------------
rsmith wrote:
> Is there somewhere more central you can do this, rather than repeating it
> once for each kind of declaration? (Eg, `PushOnScopeChains`)
That would be sane. I'll check that.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:16296
} else {
+ if (TUK == TUK_Definition)
+ warnOnReservedIdentifier(New);
----------------
rsmith wrote:
> Why do we not diagnose the other possible `TagUseKind`s? `struct _foo;` and
> `struct _foo *p;` both use reserved identifiers too.
We have a test case for `struct _foo` and its correctly diagnosed. I'll double
check for pointer / reference 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
