serge-sans-paille updated this revision to Diff 319929.
serge-sans-paille added a comment.

Back to the previous version, as suggested by @rsmith . I made a few updates to 
`NamedDecl::isReserved` which get me close to the expected result, without too 
much overhead.


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

https://reviews.llvm.org/D93095

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Sema/reserved-identifier.c
  clang/test/Sema/reserved-identifier.cpp

Index: clang/test/Sema/reserved-identifier.cpp
===================================================================
--- /dev/null
+++ clang/test/Sema/reserved-identifier.cpp
@@ -0,0 +1,79 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify -Wreserved-identifier %s
+
+int foo__bar() { return 0; }    // expected-warning {{'foo__bar' is a reserved identifier}}
+static int _bar() { return 0; } // expected-warning {{'_bar' is a reserved identifier}}
+static int _Bar() { return 0; } // expected-warning {{'_Bar' is a reserved identifier}}
+int _barbouille() { return 0; } // expected-warning {{'_barbouille' is a reserved identifier}}
+
+void foo(unsigned int _Reserved) { // expected-warning {{'_Reserved' is a reserved identifier}}
+  unsigned int __1 =               // expected-warning {{'__1' is a reserved identifier}}
+      _Reserved;                   // no-warning
+}
+
+// This one is explicitly skipped by -Wreserved-identifier
+void *_; // no-warning
+
+template <class T> constexpr bool __toucan = true; // expected-warning {{'__toucan' is a reserved identifier}}
+
+template <class T>
+concept _Barbotine = __toucan<T>; // expected-warning {{'_Barbotine' is a reserved identifier}}
+
+template <class __> // expected-warning {{'__' is a reserved identifier}}
+struct BarbeNoire {};
+
+template <class __> // expected-warning {{'__' is a reserved identifier}}
+void BarbeRousse() {}
+
+namespace _Barbidur { // expected-warning {{'_Barbidur' is a reserved identifier}}
+
+struct __barbidou {}; // expected-warning {{'__barbidou' is a reserved identifier}}
+struct _barbidou {};  // no-warning
+
+int __barbouille; // expected-warning {{'__barbouille' is a reserved identifier}}
+int _barbouille;  // no-warning
+
+int __babar() { return 0; } // expected-warning {{'__babar' is a reserved identifier}}
+int _babar() { return 0; }  // no-warning
+
+} // namespace _Barbidur
+
+class __barbapapa {     // expected-warning {{'__barbapapa' is a reserved identifier}}
+  void _barbabelle() {} // no-warning
+  int _Barbalala;       // expected-warning {{'_Barbalala' is a reserved identifier}}
+};
+
+enum class __menu { // expected-warning {{'__menu' is a reserved identifier}}
+  __some,           // expected-warning {{'__some' is a reserved identifier}}
+  _Other,           // expected-warning {{'_Other' is a reserved identifier}}
+  _other            // no-warning
+};
+
+enum _Menu { // expected-warning {{'_Menu' is a reserved identifier}}
+  _OtheR_,   // expected-warning {{'_OtheR_' is a reserved identifier}}
+  _other_    // expected-warning {{'_other_' is a reserved identifier}}
+};
+
+enum {
+  __some, // expected-warning {{'__some' is a reserved identifier}}
+  _Other, // expected-warning {{'_Other' is a reserved identifier}}
+  _other  // expected-warning {{'_other' is a reserved identifier}}
+};
+
+static union {
+  int _barbeFleurie; // no-warning
+};
+
+using _Barbamama = __barbapapa; // expected-warning {{'_Barbamama' is a reserved identifier}}
+
+int foobar() {
+  return foo__bar(); // no-warning
+}
+
+namespace {
+int _barbatruc; // no-warning
+}
+
+long double operator"" _BarbeBleue(long double) // no-warning
+{
+  return 0.;
+}
Index: clang/test/Sema/reserved-identifier.c
===================================================================
--- /dev/null
+++ clang/test/Sema/reserved-identifier.c
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wreserved-identifier -Wno-visibility %s
+
+#define __oof foo__ // expected-warning {{macro name is a reserved identifier}}
+
+int foo__bar() { return 0; }    // no-warning
+static int _bar() { return 0; } // expected-warning {{'_bar' is a reserved identifier}}
+static int _Bar() { return 0; } // expected-warning {{'_Bar' is a reserved identifier}}
+int _foo() { return 0; }        // expected-warning {{'_foo' is a reserved identifier}}
+
+// This one is explicitly skipped by -Wreserved-identifier
+void *_; // no-warning
+
+void foo(unsigned int _Reserved) { // expected-warning {{'_Reserved' is a reserved identifier}}
+  unsigned int __1 =               // expected-warning {{'__1' is a reserved identifier}}
+      _Reserved;                   // no-warning
+  goto __reserved;
+__reserved: // expected-warning {{'__reserved' is a reserved identifier}}
+            ;
+}
+
+enum __menu { // expected-warning {{'__menu' is a reserved identifier}}
+  __some,     // expected-warning {{'__some' is a reserved identifier}}
+  _Other,     // expected-warning {{'_Other' is a reserved identifier}}
+  _other      // expected-warning {{'_other' is a reserved identifier}}
+};
+
+struct __babar { // expected-warning {{'__babar' is a reserved identifier}}
+};
+
+struct _Zebulon; // expected-warning {{'_Zebulon' is a reserved identifier}}
+
+typedef struct {
+  int _Field; // expected-warning {{'_Field' is a reserved identifier}}
+  int _field; // no-warning
+} _Typedef;   // expected-warning {{'_Typedef' is a reserved identifier}}
+
+int foobar() {
+  return foo__bar(); // no-warning
+}
+
+struct _reserved { // expected-warning {{'_reserved' is a reserved identifier}}
+  int a;
+} cunf(void) {
+  return (struct _reserved){1};
+}
+
+// FIXME: According to clang declaration context layering, _preserved belongs to
+// the translation unit, so we emit a warning. It's unclear that's what the
+// standard mandate, but it's such a corner case we can live with it.
+void func(struct _preserved { int a; } r) {} // expected-warning {{'_preserved' is a reserved identifier}}
+
+extern char *_strdup(const char *); // expected-warning {{'_strdup' is a reserved identifier}}
+
+// Don't warn on redecleration
+extern char *_strdup(const char *); // no-warning
Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1677,6 +1677,9 @@
   if (ExportLoc.isValid())
     Diag(ExportLoc, diag::warn_template_export_unsupported);
 
+  for (NamedDecl *P : Params)
+    warnOnReservedIdentifier(P);
+
   return TemplateParameterList::Create(
       Context, TemplateLoc, LAngleLoc,
       llvm::makeArrayRef(Params.data(), Params.size()),
@@ -8628,6 +8631,7 @@
   }
 
   ActOnDocumentableDecl(NewDecl);
+  warnOnReservedIdentifier(NewDecl);
   PushOnScopeChains(NewDecl, S);
   return NewDecl;
 }
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -541,6 +541,10 @@
     return SubStmt;
   }
 
+  if (!Context.getSourceManager().isInSystemHeader(IdentLoc) &&
+      TheDecl->isReserved(getLangOpts()))
+    Diag(IdentLoc, diag::warn_reserved_identifier) << TheDecl;
+
   // Otherwise, things are good.  Fill in the declaration and return it.
   LabelStmt *LS = new (Context) LabelStmt(IdentLoc, TheDecl, SubStmt);
   TheDecl->setStmt(LS);
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -10015,8 +10015,9 @@
   DeclContext *LookupDC = dyn_cast<DeclContext>(D);
 
   if (DeclaratorDecl *DD = dyn_cast<DeclaratorDecl>(D)) {
-    for (unsigned i = 0; i < DD->getNumTemplateParameterLists(); ++i)
+    for (unsigned i = 0; i < DD->getNumTemplateParameterLists(); ++i) {
       ParameterLists.push_back(DD->getTemplateParameterList(i));
+    }
 
     if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
       if (FunctionTemplateDecl *FTD = FD->getDescribedFunctionTemplate())
@@ -10988,6 +10989,9 @@
   // for the namespace has the declarations that showed up in that particular
   // namespace definition.
   PushDeclContext(NamespcScope, Namespc);
+
+  warnOnReservedIdentifier(Namespc);
+
   return Namespc;
 }
 
@@ -12662,6 +12666,9 @@
 
   PushOnScopeChains(NewND, S);
   ActOnDocumentableDecl(NewND);
+
+  warnOnReservedIdentifier(NewND);
+
   return NewND;
 }
 
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5549,6 +5549,16 @@
   return false;
 }
 
+void Sema::warnOnReservedIdentifier(const NamedDecl *D) {
+  // Avoid warning twice on the same identifier, and don't warn on redeclaration
+  // of system decl
+  if(D->getPreviousDecl())
+    return;
+  if (!Context.getSourceManager().isInSystemHeader(D->getLocation()) &&
+      D->isReserved(getLangOpts()))
+    Diag(D->getLocation(), diag::warn_reserved_identifier) << D;
+}
+
 Decl *Sema::ActOnDeclarator(Scope *S, Declarator &D) {
   D.setFunctionDefinitionKind(FunctionDefinitionKind::Declaration);
   Decl *Dcl = HandleDeclarator(S, D, MultiTemplateParamsArg());
@@ -5894,6 +5904,7 @@
   if (isInOpenMPDeclareTargetContext())
     checkDeclIsAllowedInOpenMPTarget(nullptr, New);
 
+  warnOnReservedIdentifier(New);
   return New;
 }
 
@@ -13631,6 +13642,8 @@
   if (getLangOpts().OpenCL)
     deduceOpenCLAddressSpace(New);
 
+  warnOnReservedIdentifier(New);
+
   return New;
 }
 
@@ -16285,6 +16298,7 @@
   } else if (SkipBody && SkipBody->ShouldSkip) {
     return SkipBody->Previous;
   } else {
+    warnOnReservedIdentifier(New);
     return New;
   }
 }
@@ -17097,6 +17111,8 @@
        i != end; ++i) {
     FieldDecl *FD = cast<FieldDecl>(*i);
 
+    warnOnReservedIdentifier(FD);
+
     // Get the type for the field.
     const Type *FDTy = FD->getType().getTypePtr();
 
@@ -17822,6 +17838,8 @@
 
   ActOnDocumentableDecl(New);
 
+  warnOnReservedIdentifier(New);
+
   return New;
 }
 
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -1078,6 +1078,40 @@
   return L == getCachedLinkage();
 }
 
+bool NamedDecl::isReserved(const LangOptions &LangOpts) const {
+  IdentifierInfo *II = getIdentifier();
+  if (!II)
+    return false;
+
+  StringRef Name = II->getName();
+  // '_' is a reserved identifier, but it's use is so common (e.g. to store
+  // ignored values) that we don't warn on it.
+  if (Name.size() <= 1)
+    return false;
+
+  // [lex.name] p3
+  if (Name[0] == '_') {
+
+    // 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();
+    if (isa<TranslationUnitDecl>(DC))
+      return true;
+
+    // Each name that begins with an underscore followed by an uppercase letter
+    // or another underscore is reserved.
+    if (Name[1] == '_' || ('A' <= Name[1] && Name[1] <= 'Z'))
+      return true;
+  }
+
+  // Each name that contains a double underscore (__) is reserved.
+  if (LangOpts.CPlusPlus && Name.contains("__"))
+    return true;
+
+  return false;
+}
+
 ObjCStringFormatFamily NamedDecl::getObjCFStringFormattingFamily() const {
   StringRef name = getName();
   if (name.empty()) return SFF_None;
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2572,6 +2572,8 @@
                                           SourceLocation Less,
                                           SourceLocation Greater);
 
+  void warnOnReservedIdentifier(const NamedDecl *D);
+
   Decl *ActOnDeclarator(Scope *S, Declarator &D);
 
   NamedDecl *HandleDeclarator(Scope *S, Declarator &D,
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -372,6 +372,10 @@
   "%select{used|required to be captured for this use}1">,
   InGroup<UnusedLambdaCapture>, DefaultIgnore;
 
+def warn_reserved_identifier: Warning<
+  "%0 is a reserved identifier">,
+  InGroup<ReservedIdentifier>, DefaultIgnore;
+
 def warn_parameter_size: Warning<
   "%0 is a large (%1 bytes) pass-by-value argument; "
   "pass it by reference instead ?">, InGroup<LargeByValueCopy>;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -773,6 +773,9 @@
 def DuplicateArgDecl : DiagGroup<"duplicate-method-arg">;
 def SignedEnumBitfield : DiagGroup<"signed-enum-bitfield">;
 
+
+def ReservedIdentifier : DiagGroup<"reserved-identifier", [ReservedIdAsMacro]>;
+
 // Unreachable code warning groups.
 //
 //  The goal is make -Wunreachable-code on by default, in -Wall, or at
Index: clang/include/clang/AST/Decl.h
===================================================================
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -356,6 +356,10 @@
   /// a C++ class.
   bool isCXXInstanceMember() const;
 
+  /// Determine if the declaration obeys the reserved identifier rules of the
+  /// given language.
+  bool isReserved(const LangOptions &) const;
+
   /// Determine what kind of linkage this entity has.
   ///
   /// This is not the linkage as defined by the standard or the codegen notion
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to