[PATCH] D93095: Introduce -Wreserved-identifier

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman 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.
Sorry @serge-sans-paille for my think-o, Richard is right.

My thinking was "if the declaration can be found at the global namespace, then it's reserved", but.. that's a hypothetical declaration, not an actual one, so of course the name lookup won't find it. Derp.

I guess we really do have to look at the `DeclContext`.

@rsmith -- just to be clear, from http://eel.is/c++draft/lex.name#3.2, does "for use as a name in the global namespace" imply macros as well? e.g., can an implementation decide to use this reservation to `#define _foo 12`, which would cause problems with a `_foo` declared in a user-defined namespace? If so, then we can ignore what the declaration context is because the name could be snatched out from under the user via a macro anyway.


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

https://reviews.llvm.org/D93095