[PATCH] D93095: Introduce -Wreserved-identifier

[serge via Phabricator via cfe-commits]

serge-sans-paille marked 2 inline comments as done.
serge-sans-paille added inline comments.


================
Comment at: clang/test/Sema/reserved-identifier.cpp:58
+// we skip this one because it's not at top-level.
+int _barbatruc; // no-warning
+}
----------------
aaron.ballman wrote:
> This is another case that I'm on the fence about failing to warn on because the name `_barbatruc` would conflict with a name introduced by the implementation. Another interesting variant of the same problem, for C++:
> ```
> static union {
>   int _field;
> };
> ```
> I wonder if the rule should not be whether the declaration is at file scope or not, but whether the declared identifier can be found at file scope or not?
> whether the declared identifier can be found at file scope or not?

100% agree. As

```
static union {
  int _field;
};
int _field;
```

is invalid, I considered that one and tested it.


================
Comment at: clang/test/Sema/reserved-identifier.cpp:40
+  return foo__bar(); // no-warning
+}
----------------
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.


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

https://reviews.llvm.org/D93095