rsmith added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:383
+ "identifier %0 is reserved because %select{"
+ "|" // passing 0 for %1 is not valid but we need that | to make the enum
order match the diagnostic
+ "it starts with '_' at global scope|"
----------------
This is hopefully more useful to future readers and also terser. (I like
including <ERROR> in the diagnostic in the bad case because if it ever happens
we're more likely to get a bug report that way.)
================
Comment at: clang/lib/AST/Decl.cpp:1084
+ const IdentifierInfo *II = nullptr;
+ if (const auto *FD = dyn_cast<FunctionDecl>(this))
+ II = FD->getLiteralIdentifier();
----------------
Please reorder the literal operator case after the plain `getIdentifier` case.
I think this'd be more readable if the common and expected case is first, and
it doesn't make any functional difference since a literal operator doesn't have
a "regular" identifier name.
================
Comment at: clang/lib/AST/Decl.cpp:1097
+ // ignored values) that we don't warn on it.
+ if (Name.size() <= 1)
+ return ReservedIdentifierStatus::NotReserved;
----------------
Would it make sense to move the rest of this function to a member on
`IdentifierInfo`? That is, the rest of this function would become:
```
ReservedIdentifierStatus Status = II->isReserved(LangOpts);
if (Status == ReservedIdentifierStatus::StartsWithUnderscoreAtGlobalScope &&
isa<ParmVarDecl>(this) || isTemplateParameter() ||
!getDeclContext()->getRedeclContext()->isTranslationUnit())
return ReservedIdentifierStatus::NotReserved;
return Status;
```
That way, we should be able to reuse the same implementation to detect reserved
macros too.
================
Comment at: clang/lib/AST/Decl.cpp:1113-1119
+ if (!isa<ParmVarDecl>(this) && !isTemplateParameter()) {
+ const DeclContext *DC = getLexicalDeclContext();
+ while (DC->isTransparentContext())
+ DC = DC->getLexicalParent();
+ if (isa<TranslationUnitDecl>(DC))
+ return ReservedIdentifierStatus::StartsWithUnderscoreAtGlobalScope;
+ }
----------------
This can be simplified a bit; I suggested edits above. Also, we want the
semantic parent here, not the lexical parent, in order to warn on cases like
```
struct A {
friend struct _b; // warning, starts with underscore at global scope
friend void _f(); // warning, starts with underscore at global scope
};
void g() {
void _h(); // warning, starts with underscore at global scope
extern int _n; // warning, starts with underscore at global scope
}
```
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
