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)) ---------------- aaron.ballman wrote: > 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. Regarding macros, I think it depends what you mean. The implementation is permitted to declare additional `_foo` names in the global namespace, and I think the intent is that these can be either predeclared or declared in headers. The user is permitted to declare `_foo` names in other (non-reserved) namespaces. As a result, neither party should define an `_foo` macro, because doing so would step on the other party's namespace (though it would be OK for user code to do so after all of their standard library includes, I suppose). Specifically, I think it would be non-conforming for the implementation to `#define` such a macro (because the name is not reserved to the implementation in all contexts), and I think it would be UB via [macro.names]/1 for user code to `#define` such a macro that actually collided with a name that happened to be declared in the global namespace by a standard library header. We can probably warn on `#define _foo` if we want to, but I don't think we can produce a `-pedantic-error` for that case, because I think it's (sometimes) valid for user code to do that. 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