[clang] 3cd70fc - Detect diagnostic groups that are defined in multiple 'def's.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 18 17:19:14 PST 2021


Author: Richard Smith
Date: 2021-02-18T17:19:01-08:00
New Revision: 3cd70fc59d2a1a907d14bbf3f2dfe53d2c2be9d1

URL: https://github.com/llvm/llvm-project/commit/3cd70fc59d2a1a907d14bbf3f2dfe53d2c2be9d1
DIFF: https://github.com/llvm/llvm-project/commit/3cd70fc59d2a1a907d14bbf3f2dfe53d2c2be9d1.diff

LOG: Detect diagnostic groups that are defined in multiple 'def's.

Remove the three such groups that we've accumulated. These were causing
duplicated output to appear in generated the diagnostic reference.

Added: 
    clang/test/TableGen/redefined-group.td

Modified: 
    clang/include/clang/Basic/DiagnosticGroups.td
    clang/utils/TableGen/ClangDiagnosticsEmitter.cpp

Removed: 
    clang/test/TableGen/anonymous-groups.td
    clang/test/TableGen/tg-fixits.td


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 21fc7d4cb82b..1a1ce66656f5 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -505,7 +505,6 @@ def PointerArith : DiagGroup<"pointer-arith">;
 def PoundWarning : DiagGroup<"#warnings">;
 def PoundPragmaMessage : DiagGroup<"#pragma-messages">,
                          DiagCategory<"#pragma message Directive">;
-def : DiagGroup<"pointer-to-int-cast">;
 def : DiagGroup<"redundant-decls">;
 def RedeclaredClassMember : DiagGroup<"redeclared-class-member">;
 def GNURedeclaredEnum : DiagGroup<"gnu-redeclared-enum">;
@@ -540,7 +539,6 @@ def ShadowAll : DiagGroup<"shadow-all", [Shadow, ShadowFieldInConstructor,
 def Shorten64To32 : DiagGroup<"shorten-64-to-32">;
 def : DiagGroup<"sign-promo">;
 def SignCompare : DiagGroup<"sign-compare">;
-def : DiagGroup<"stack-protector">;
 def : DiagGroup<"switch-default">;
 def : DiagGroup<"synth">;
 def SizeofArrayArgument : DiagGroup<"sizeof-array-argument">;
@@ -626,7 +624,6 @@ def : DiagGroup<"strict-overflow=5">;
 def : DiagGroup<"strict-overflow">;
 
 def InvalidOffsetof : DiagGroup<"invalid-offsetof">;
-def : DiagGroup<"strict-prototypes">;
 def StrictSelector : DiagGroup<"strict-selector-match">;
 def MethodDuplicate : DiagGroup<"duplicate-method-match">;
 def ObjCCStringFormat : DiagGroup<"cstring-format-directive">;

diff  --git a/clang/test/TableGen/anonymous-groups.td b/clang/test/TableGen/anonymous-groups.td
deleted file mode 100644
index 9e8dc398bb24..000000000000
--- a/clang/test/TableGen/anonymous-groups.td
+++ /dev/null
@@ -1,34 +0,0 @@
-// RUN: clang-tblgen -gen-clang-diag-groups -I%S %s -o /dev/null 2>&1 | FileCheck --strict-whitespace %s
-include "DiagnosticBase.inc"
-
-// Do not move this line; it is referred to by absolute line number in the
-// FileCheck lines below.
-def NamedGroup : DiagGroup<"name">;
-
-
-def InNamedGroup : Warning<"">, InGroup<DiagGroup<"name">>;
-//      CHECK: anonymous-groups.td:[[@LINE-1]]:1: error: group 'name' is referred to anonymously
-// CHECK-NEXT: {{^def InNamedGroup : Warning<"">, InGroup<DiagGroup<"name">>;}}
-//      CHECK: anonymous-groups.td:6:1: note: group defined here
-// CHECK-NEXT: def NamedGroup : DiagGroup<"name">;
-// CHECK-NEXT: ^
-
-
-def AlsoInNamedGroup : Warning<"">, InGroup  < DiagGroup<"name"> >;
-//      CHECK: anonymous-groups.td:[[@LINE-1]]:1: error: group 'name' is referred to anonymously
-// CHECK-NEXT: {{^def AlsoInNamedGroup : Warning<"">, InGroup  < DiagGroup<"name"> >;}}
-//      CHECK: anonymous-groups.td:6:1: note: group defined here
-// CHECK-NEXT: def NamedGroup : DiagGroup<"name">;
-// CHECK-NEXT: ^
-
-
-def AnonymousGroup : Warning<"">, InGroup<DiagGroup<"anonymous">>;
-def AlsoAnonymousGroup : Warning<"">, InGroup<DiagGroup<"anonymous">>;
-def AnonymousGroupAgain : Warning<"">,
-  InGroup<DiagGroup<"anonymous">>;
-
-//      CHECK: anonymous-groups.td:[[@LINE-5]]:1: error: group 'anonymous' is referred to anonymously
-// CHECK-NEXT: {{^def AnonymousGroup : Warning<"">, InGroup<DiagGroup<"anonymous">>;}}
-//      CHECK: anonymous-groups.td:[[@LINE-6]]:1: note: also referenced here
-// CHECK-NEXT: {{^def AlsoAnonymousGroup : Warning<"">, InGroup<DiagGroup<"anonymous">>;}}
-//      CHECK: anonymous-groups.td:[[@LINE-7]]:1: note: also referenced here

diff  --git a/clang/test/TableGen/redefined-group.td b/clang/test/TableGen/redefined-group.td
new file mode 100644
index 000000000000..5ffeb3011978
--- /dev/null
+++ b/clang/test/TableGen/redefined-group.td
@@ -0,0 +1,43 @@
+// RUN: clang-tblgen -gen-clang-diag-groups -I%S %s -o /dev/null 2>&1 | FileCheck %s
+include "DiagnosticBase.inc"
+
+def NamedGroup : DiagGroup<"a">;
+def InNamedGroup1 : Warning<"">, InGroup<DiagGroup<"a">>;
+def InNamedGroup2 : Warning<"">, InGroup  < DiagGroup<"a"> >;
+// CHECK: redefined-group.td:[[@LINE-3]]:1: error: group 'a' is defined more than once
+// CHECK: redefined-group.td:[[@LINE-3]]:1: note: also implicitly defined here
+// CHECK: redefined-group.td:[[@LINE-3]]:1: note: also implicitly defined here
+
+def : DiagGroup<"b">;
+def InUnnamedGroup : Warning<"">, InGroup<DiagGroup<"b">>;
+// CHECK: redefined-group.td:[[@LINE-2]]:1: error: group 'b' is defined more than once
+// CHECK: redefined-group.td:[[@LINE-2]]:1: note: also implicitly defined here
+
+def ImplicitGroup1 : Warning<"">, InGroup<DiagGroup<"c">>;
+def ImplicitGroup2 : Warning<"">, InGroup<DiagGroup<"c">>;
+def ImplicitGroup3 : Warning<"">,
+  InGroup<DiagGroup<"c">>;
+// CHECK: redefined-group.td:[[@LINE-4]]:1: error: group 'c' is implicitly defined more than once
+// CHECK: redefined-group.td:[[@LINE-4]]:1: note: also implicitly defined here
+// CHECK: redefined-group.td:[[@LINE-4]]:1: note: also implicitly defined here
+
+def NamedAndUnnamed : DiagGroup<"d">;
+def : DiagGroup<"d">;
+// CHECK: redefined-group.td:[[@LINE-2]]:1: error: group 'd' is defined more than once
+// CHECK: redefined-group.td:[[@LINE-2]]:1: note: also defined here
+
+def : DiagGroup<"e">;
+def NamedAndUnnamed2 : DiagGroup<"e">;
+// CHECK: redefined-group.td:[[@LINE-1]]:1: error: group 'e' is defined more than once
+// CHECK: redefined-group.td:[[@LINE-3]]:1: note: also defined here
+
+def InGroupF1 : Warning<"">, InGroup<DiagGroup<"f">>;
+def : DiagGroup<"f">; // FIXME: It'd be nice to also note this, but it's hard to detect.
+def InGroupF2 : Warning<"">, InGroup<DiagGroup<"f">>;
+def GroupF : DiagGroup<"f">;
+def InGroupF3 : Warning<"">, InGroup<GroupF>;
+def InGroupF4 : Warning<"">, InGroup<DiagGroup<"f">>;
+// CHECK: redefined-group.td:[[@LINE-5]]:1: error: group 'f' is defined more than once
+// CHECK: redefined-group.td:[[@LINE-7]]:1: note: also implicitly defined here
+// CHECK: redefined-group.td:[[@LINE-6]]:1: note: also implicitly defined here
+// CHECK: redefined-group.td:[[@LINE-4]]:1: note: also implicitly defined here

diff  --git a/clang/test/TableGen/tg-fixits.td b/clang/test/TableGen/tg-fixits.td
deleted file mode 100644
index f0f62ef9a4be..000000000000
--- a/clang/test/TableGen/tg-fixits.td
+++ /dev/null
@@ -1,25 +0,0 @@
-// RUN: clang-tblgen -gen-clang-diag-groups -I%S %s -o /dev/null 2>&1 | FileCheck --strict-whitespace %s
-include "DiagnosticBase.inc"
-
-def NamedGroup : DiagGroup<"name">;
-
-def InNamedGroup : Warning<"">, InGroup<DiagGroup<"name">>;
-//      CHECK: tg-fixits.td:[[@LINE-1]]:1: error: group 'name' is referred to anonymously
-// CHECK-NEXT: {{^def InNamedGroup : Warning<"">, InGroup<DiagGroup<"name">>;}}
-
-def Wrapped : Warning<"">, InGroup<DiagGroup<
-  "name">>;
-//      CHECK: tg-fixits.td:[[@LINE-2]]:1: error: group 'name' is referred to anonymously
-// CHECK-NEXT: {{^def Wrapped : Warning<"">, InGroup<DiagGroup<}}
-
-def AlsoWrapped : Warning<"">, InGroup<
-  DiagGroup<"name">>;
-//      CHECK: tg-fixits.td:[[@LINE-2]]:1: error: group 'name' is referred to anonymously
-
-// The following line has Unicode characters in it; do not change them!
-// FIXME: For now, we just give up on printing carets/ranges/fixits for
-// lines with Unicode in them, because SMDiagnostic don't keep a byte<->column
-// map around to line things up like Clang does.
-def Unicode : Warning<"ユニコード">, InGroup<DiagGroup<"name">>;
-//      CHECK: tg-fixits.td:[[@LINE-1]]:1: error: group 'name' is referred to anonymously
-// CHECK-NEXT: def Unicode : Warning<"{{[^"]+}}">, InGroup<DiagGroup<"name">>;

diff  --git a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
index 430895d8425f..77c478672324 100644
--- a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -133,9 +133,9 @@ namespace {
     std::vector<std::string> SubGroups;
     unsigned IDNo;
 
-    const Record *ExplicitDef;
+    llvm::SmallVector<const Record *, 1> Defs;
 
-    GroupInfo() : IDNo(0), ExplicitDef(nullptr) {}
+    GroupInfo() : IDNo(0) {}
   };
 } // end anonymous namespace.
 
@@ -150,12 +150,6 @@ static bool diagGroupBeforeByName(const Record *LHS, const Record *RHS) {
          RHS->getValueAsString("GroupName");
 }
 
-static bool beforeThanCompareGroups(const GroupInfo *LHS, const GroupInfo *RHS){
-  assert(!LHS->DiagsInGroup.empty() && !RHS->DiagsInGroup.empty());
-  return beforeThanCompare(LHS->DiagsInGroup.front(),
-                           RHS->DiagsInGroup.front());
-}
-
 /// Invert the 1-[0/1] mapping of diags to group into a one to many
 /// mapping of groups to diags in the group.
 static void groupDiagnostics(const std::vector<Record*> &Diags,
@@ -174,24 +168,13 @@ static void groupDiagnostics(const std::vector<Record*> &Diags,
     DiagsInGroup[GroupName].DiagsInGroup.push_back(R);
   }
 
-  typedef SmallPtrSet<GroupInfo *, 16> GroupSetTy;
-  GroupSetTy ImplicitGroups;
-
   // Add all DiagGroup's to the DiagsInGroup list to make sure we pick up empty
   // groups (these are warnings that GCC supports that clang never produces).
   for (unsigned i = 0, e = DiagGroups.size(); i != e; ++i) {
     Record *Group = DiagGroups[i];
     GroupInfo &GI =
         DiagsInGroup[std::string(Group->getValueAsString("GroupName"))];
-    if (Group->isAnonymous()) {
-      if (GI.DiagsInGroup.size() > 1)
-        ImplicitGroups.insert(&GI);
-    } else {
-      if (GI.ExplicitDef)
-        assert(GI.ExplicitDef == Group);
-      else
-        GI.ExplicitDef = Group;
-    }
+    GI.Defs.push_back(Group);
 
     std::vector<Record*> SubGroups = Group->getValueAsListOfDefs("SubGroups");
     for (unsigned j = 0, e = SubGroups.size(); j != e; ++j)
@@ -205,61 +188,51 @@ static void groupDiagnostics(const std::vector<Record*> &Diags,
        I = DiagsInGroup.begin(), E = DiagsInGroup.end(); I != E; ++I, ++IDNo)
     I->second.IDNo = IDNo;
 
-  // Sort the implicit groups, so we can warn about them deterministically.
-  SmallVector<GroupInfo *, 16> SortedGroups(ImplicitGroups.begin(),
-                                            ImplicitGroups.end());
-  for (SmallVectorImpl<GroupInfo *>::iterator I = SortedGroups.begin(),
-                                              E = SortedGroups.end();
-       I != E; ++I) {
-    MutableArrayRef<const Record *> GroupDiags = (*I)->DiagsInGroup;
-    llvm::sort(GroupDiags, beforeThanCompare);
-  }
-  llvm::sort(SortedGroups, beforeThanCompareGroups);
+  // Warn if the same group is defined more than once (including implicitly).
+  for (auto &Group : DiagsInGroup) {
+    if (Group.second.Defs.size() == 1 &&
+        (!Group.second.Defs.front()->isAnonymous() ||
+         Group.second.DiagsInGroup.size() <= 1))
+      continue;
 
-  // Warn about the same group being used anonymously in multiple places.
-  for (SmallVectorImpl<GroupInfo *>::const_iterator I = SortedGroups.begin(),
-                                                    E = SortedGroups.end();
-       I != E; ++I) {
-    ArrayRef<const Record *> GroupDiags = (*I)->DiagsInGroup;
-
-    if ((*I)->ExplicitDef) {
-      std::string Name =
-          std::string((*I)->ExplicitDef->getValueAsString("GroupName"));
-      for (ArrayRef<const Record *>::const_iterator DI = GroupDiags.begin(),
-                                                    DE = GroupDiags.end();
-           DI != DE; ++DI) {
-        const DefInit *GroupInit = cast<DefInit>((*DI)->getValueInit("Group"));
-        const Record *NextDiagGroup = GroupInit->getDef();
-        if (NextDiagGroup == (*I)->ExplicitDef)
-          continue;
-
-        SrcMgr.PrintMessage((*DI)->getLoc().front(),
-                            SourceMgr::DK_Error,
-                            Twine("group '") + Name +
-                              "' is referred to anonymously");
-        SrcMgr.PrintMessage((*I)->ExplicitDef->getLoc().front(),
-                            SourceMgr::DK_Note, "group defined here");
+    bool First = true;
+    for (const Record *Def : Group.second.Defs) {
+      // Skip implicit definitions from diagnostics; we'll report those
+      // separately below.
+      bool IsImplicit = false;
+      for (const Record *Diag : Group.second.DiagsInGroup) {
+        if (cast<DefInit>(Diag->getValueInit("Group"))->getDef() == Def) {
+          IsImplicit = true;
+          break;
+        }
       }
-    } else {
-      // If there's no existing named group, we should just warn once and use
-      // notes to list all the other cases.
-      ArrayRef<const Record *>::const_iterator DI = GroupDiags.begin(),
-                                               DE = GroupDiags.end();
-      assert(DI != DE && "We only care about groups with multiple uses!");
-
-      const DefInit *GroupInit = cast<DefInit>((*DI)->getValueInit("Group"));
-      const Record *NextDiagGroup = GroupInit->getDef();
-      std::string Name =
-          std::string(NextDiagGroup->getValueAsString("GroupName"));
-
-      SrcMgr.PrintMessage((*DI)->getLoc().front(),
-                          SourceMgr::DK_Error,
-                          Twine("group '") + Name +
-                            "' is referred to anonymously");
-
-      for (++DI; DI != DE; ++DI) {
-        SrcMgr.PrintMessage((*DI)->getLoc().front(),
-                            SourceMgr::DK_Note, "also referenced here");
+      if (IsImplicit)
+        continue;
+
+      llvm::SMLoc Loc = Def->getLoc().front();
+      if (First) {
+        SrcMgr.PrintMessage(Loc, SourceMgr::DK_Error,
+                            Twine("group '") + Group.first +
+                                "' is defined more than once");
+        First = false;
+      } else {
+        SrcMgr.PrintMessage(Loc, SourceMgr::DK_Note, "also defined here");
+      }
+    }
+
+    for (const Record *Diag : Group.second.DiagsInGroup) {
+      if (!cast<DefInit>(Diag->getValueInit("Group"))->getDef()->isAnonymous())
+        continue;
+
+      llvm::SMLoc Loc = Diag->getLoc().front();
+      if (First) {
+        SrcMgr.PrintMessage(Loc, SourceMgr::DK_Error,
+                            Twine("group '") + Group.first +
+                                "' is implicitly defined more than once");
+        First = false;
+      } else {
+        SrcMgr.PrintMessage(Loc, SourceMgr::DK_Note,
+                            "also implicitly defined here");
       }
     }
   }


        


More information about the cfe-commits mailing list