aaron.ballman added inline comments.

================
Comment at: clang/lib/AST/Decl.cpp:1099
+    const DeclContext *CurrentContext = getDeclContext();
+    while (true) {
+      if (isa<TranslationUnitDecl>(CurrentContext))
----------------
Rather than trying to manually decide whether the name will be exposed at the 
top level, I wonder if we should use `Sema::LookupName()` to try to find the 
name in the global scope. That would move this code from `AST` into `Sema` to 
avoid layering issues, so it'd be something more like `bool 
Sema::IsDeclReserved(const NamedDecl *ND) const` or `bool 
Sema::IsIdentifierReserved(const IdentifierInfo *II) const`.

The idea is to perform the lookup in global scope and if the lookup finds the 
identifier, then regardless of how it got there (anonymous union field, 
anonymous namespaces, enumeration constants, etc), we know it may conflict with 
an external declaration and diagnose.

WDYT?


================
Comment at: clang/test/Sema/reserved-identifier.cpp:40
+  return foo__bar(); // no-warning
+}
----------------
hubert.reinterpretcast wrote:
> serge-sans-paille wrote:
> > aaron.ballman wrote:
> > > aaron.ballman wrote:
> > > > You should also have some tests for:
> > > > ```
> > > > template <typename T>
> > > > void _Foobar(); // Even though it's not instantiated, it's still 
> > > > reserved.
> > > > 
> > > > template <typename _Ty> // Reserved
> > > > void whatever();
> > > > 
> > > > void func() {
> > > >   int array[10];
> > > >   for (auto _A : array) // Reserved
> > > >     ;
> > > > }
> > > > 
> > > > class _C { // Reserved
> > > > public:
> > > >   _C(); // Not reserved
> > > > };
> > > > 
> > > > unsigned operator "" huttah(unsigned long long); // Reserved 
> > > > (http://eel.is/c++draft/usrlit.suffix#1)
> > > > 
> > > > unsigned operator "" _W(unsigned long long); // Reserved
> > > > unsigned operator "" _w(unsigned long long); // Reserved
> > > > 
> > > > static unsigned operator "" _X(unsigned long long); // Not reserved
> > > > static unsigned operator "" _x(unsigned long long); // Not reserved
> > > > ```
> > > I think some of these tests are still missing. I'm especially worried 
> > > about the user-defined literal cases being diagnosed, as we'd be warning 
> > > to not do the exact thing users are expected to do.
> > User defined literal tested below and behave as expected.
> @aaron.ballman, the "not reserved" comment re: `_X` for the literal operator 
> using the `operator string-literal identifier` form above seems suspect to 
> me. See `_Bq` example in [over.literal].
Thanks, @hubert.reinterpretcast, you're correct -- I forgot just how hard we 
made the rules for UDLs when we decided to require a leading underscore. :-(


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

Reply via email to