[PATCH] D93095: Introduce -Wreserved-identifier

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/AST/Decl.h:82
+enum ReservedIdentifierReason {
+  StartsWithUnderscoreAtGlobalScope,
+  StartsWithDoubleUnderscore,
----------------
Because this is not a scoped enum, should these enumerators get an `RIR_` prefix added to them?


================
Comment at: clang/include/clang/AST/Decl.h:368-369
+  /// given language.
+  bool isReserved(const LangOptions &,
+                  ReservedIdentifierReason *Reason = nullptr) const;
+
----------------
Any reason not to return the enumeration as the result? We could add `NotReserved = 0` to the enumeration so that calls to `D->isReserved()` will do the right thing.

(Also, might as well name the `LangOptions` parameter here.)


================
Comment at: clang/include/clang/Basic/DiagnosticGroups.td:790
+
+def ReservedIdAsSymbol : DiagGroup<"reserved-extern-identifier">;
+def ReservedIdentifier : DiagGroup<"reserved-identifier",
----------------
Were you planning to use this new diagnostic group?


================
Comment at: clang/lib/AST/Decl.cpp:1084
+  const IdentifierInfo *II = nullptr;
+  if (auto *FD = dyn_cast<FunctionDecl>(this))
+    II = FD->getLiteralIdentifier();
----------------
Might as well address the lint warning.


================
Comment at: clang/lib/AST/Decl.cpp:1104-1105
+    // Walk up the lexical parents to determine if we're at TU level or not.
+    const DeclContext * DC = getLexicalDeclContext();
+    while(DC->isTransparentContext())
+      DC = DC->getLexicalParent();
----------------



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

https://reviews.llvm.org/D93095