[clang] [diagnostics] Stable IDs for Clang diagnostics

SARIF diagnostics require that each rule have a stable `id` property to identify that rule across runs, even when the compiler or analysis tool has changed. We were previously setting the `id` property to the numeric value of the enum value for that diagnostic within the Clang implementation; this value changes whenever an unrelated diagnostic is inserted or removed earlier in the list.

This change sets the `id` property to the _text_ of that same enum value. This value would only change if someone renames the enum value for that diagnostic, which should happen much less frequently than renumbering.

For now, we will just assume that renaming happens infrequently enough that existing consumers of SARIF will not notice. In the future, we could take advantage of SARIF's support for `deprecatedIds`, which let a rule specify the IDs by which it was previously known. This would let us rename, split, or combine diagnostics while still being able to correlate the new diagnostic IDs with older SARIF logs and/or suppressions.

Nothing in this change affects how warnings are configured on the command line or in `#pragma clang diagnostic`. Those still use warning groups, not the stable IDs.

Author: dbartol

clang/include/clang/Basic/DiagnosticIDs.h

@@ -332,6 +332,9 @@ class DiagnosticIDs : public RefCountedBase<DiagnosticIDs> {
   /// Given a diagnostic ID, return a description of the issue.
   StringRef getDescription(unsigned DiagID) const;
 
+  /// Given a diagnostic ID, return the stable ID of the diagnostic.
+  std::string getStableID(unsigned DiagID) const;
+
   /// Return true if the unmapped diagnostic levelof the specified
   /// diagnostic ID is a Warning or Extension.
   ///

clang/lib/Basic/DiagnosticIDs.cpp

@@ -51,6 +51,22 @@ const StaticDiagInfoDescriptionStringTable StaticDiagInfoDescriptions = {
 #undef DIAG
 };
 
+struct StaticDiagInfoStableIDStringTable {
+#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR,     \
+             SHOWINSYSHEADER, SHOWINSYSMACRO, DEFERRABLE, CATEGORY)            \
+  char ENUM##_id[sizeof(#ENUM)];
+#include "clang/Basic/AllDiagnosticKinds.inc"
+#undef DIAG
+};
+
+const StaticDiagInfoStableIDStringTable StaticDiagInfoStableIDs = {
+#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR,     \
+             SHOWINSYSHEADER, SHOWINSYSMACRO, DEFERRABLE, CATEGORY)            \
+  #ENUM,
+#include "clang/Basic/AllDiagnosticKinds.inc"
+#undef DIAG
+};
+
 extern const StaticDiagInfoRec StaticDiagInfo[];
 
 // Stored separately from StaticDiagInfoRec to pack better.  Otherwise,
@@ -63,6 +79,14 @@ const uint32_t StaticDiagInfoDescriptionOffsets[] = {
 #undef DIAG
 };
 
+const uint32_t StaticDiagInfoStableIDOffsets[] = {
+#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR,     \
+             SHOWINSYSHEADER, SHOWINSYSMACRO, DEFERRABLE, CATEGORY)            \
+  offsetof(StaticDiagInfoStableIDStringTable, ENUM##_id),
+#include "clang/Basic/AllDiagnosticKinds.inc"
+#undef DIAG
+};
+
 enum DiagnosticClass {
   CLASS_NOTE = DiagnosticIDs::CLASS_NOTE,
   CLASS_REMARK = DiagnosticIDs::CLASS_REMARK,
@@ -95,6 +119,7 @@ struct StaticDiagInfoRec {
   uint16_t Deferrable : 1;
 
   uint16_t DescriptionLen;
+  uint16_t StableIDLen;
 
   unsigned getOptionGroupIndex() const {
     return OptionGroupIndex;
@@ -107,6 +132,14 @@ struct StaticDiagInfoRec {
     return StringRef(&Table[StringOffset], DescriptionLen);
   }
 
+  StringRef getStableID() const {
+    size_t MyIndex = this - &StaticDiagInfo[0];
+    uint32_t StringOffset = StaticDiagInfoStableIDOffsets[MyIndex];
+    const char *Table =
+        reinterpret_cast<const char *>(&StaticDiagInfoStableIDs);
+    return StringRef(&Table[StringOffset], StableIDLen);
+  }
+
   diag::Flavor getFlavor() const {
     return Class == CLASS_REMARK ? diag::Flavor::Remark
                                  : diag::Flavor::WarningOrError;
@@ -159,7 +192,8 @@ const StaticDiagInfoRec StaticDiagInfo[] = {
       SHOWINSYSMACRO,                                                          \
       GROUP,                                                                   \
 	    DEFERRABLE,                                                              \
-      STR_SIZE(DESC, uint16_t)},
+      STR_SIZE(DESC, uint16_t),                                               \
+      STR_SIZE(#ENUM, uint16_t)},
 #include "clang/Basic/DiagnosticCommonKinds.inc"
 #include "clang/Basic/DiagnosticDriverKinds.inc"
 #include "clang/Basic/DiagnosticFrontendKinds.inc"
@@ -434,6 +468,15 @@ StringRef DiagnosticIDs::getDescription(unsigned DiagID) const {
   return CustomDiagInfo->getDescription(DiagID).GetDescription();
 }
 
+/// getIDString - Given a diagnostic ID, return the stable ID of the diagnostic.
+std::string DiagnosticIDs::getStableID(unsigned DiagID) const {
+  if (const StaticDiagInfoRec *Info = GetDiagInfo(DiagID))
+    return Info->getStableID().str();
+  assert(CustomDiagInfo && "Invalid CustomDiagInfo");
+  // TODO: Stable IDs for custom diagnostics?
+  return std::to_string(DiagID);
+}
+
 static DiagnosticIDs::Level toLevel(diag::Severity SV) {
   switch (SV) {
   case diag::Severity::Ignored:

clang/lib/Frontend/SARIFDiagnostic.cpp

@@ -46,7 +46,9 @@ void SARIFDiagnostic::emitDiagnosticMessage(
   if (!Diag)
     return;
 
-  SarifRule Rule = SarifRule::create().setRuleId(std::to_string(Diag->getID()));
+  std::string StableID =
+      Diag->getDiags()->getDiagnosticIDs()->getStableID(Diag->getID());
+  SarifRule Rule = SarifRule::create().setRuleId(StableID);
 
   Rule = addDiagnosticLevelToRule(Rule, Level);
 

clang/test/Frontend/sarif-diagnostics.cpp

@@ -34,35 +34,35 @@ void f1(t1 x, t1 y) {
 // Omit filepath to llvm project directory
 // CHECK: test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":
 // CHECK: [{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0,"uri":"file://
-// CHECK: {"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main' must return 'int'"},"ruleId":"{{[0-9]+}}","ruleIndex":0},
+// CHECK: {"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main' must return 'int'"},"ruleId":"{{[A-Za-z0-9_]+}}","ruleIndex":0},
 // CHECK: {"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0,"uri":"file://
 // CHECK: {"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use of undeclared identifier
-// CHECK: 'hello'"},"ruleId":"{{[0-9]+}}","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":
+// CHECK: 'hello'"},"ruleId":"{{[A-Za-z0-9_]+}}","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":
 // CHECK: {"index":0,"uri":"file://{{.+}}"},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid digit 'a' in decimal
-// CHECK: constant"},"ruleId":"{{[0-9]+}}","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":
+// CHECK: constant"},"ruleId":"{{[A-Za-z0-9_]+}}","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":
 // CHECK: {"index":0,"uri":"file://{{.+}}"},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading indentation; statement is not part
-// CHECK: of the previous 'if'"},"ruleId":"{{[0-9]+}}","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":
+// CHECK: of the previous 'if'"},"ruleId":"{{[A-Za-z0-9_]+}}","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":
 // CHECK: {"index":0,"uri":"file://{{.+}}"},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous statement is
-// CHECK: here"},"ruleId":"{{[0-9]+}}","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":
+// CHECK: here"},"ruleId":"{{[A-Za-z0-9_]+}}","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":
 // CHECK: {"index":0,"uri":"file://{{.+}}"},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused variable
-// CHECK: 'Yes'"},"ruleId":"{{[0-9]+}}","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0,"uri":"file://
+// CHECK: 'Yes'"},"ruleId":"{{[A-Za-z0-9_]+}}","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0,"uri":"file://
 // CHECK: {"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use of undeclared identifier
-// CHECK: 'hi'"},"ruleId":"{{[0-9]+}}","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0,"uri":"file://
+// CHECK: 'hi'"},"ruleId":"{{[A-Za-z0-9_]+}}","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0,"uri":"file://
 // CHECK: {"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous closing brace
-// CHECK: ('}')"},"ruleId":"{{[0-9]+}}","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0,"uri":"file://
+// CHECK: ('}')"},"ruleId":"{{[A-Za-z0-9_]+}}","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0,"uri":"file://
 // CHECK: {"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0,"uri":"file://
 // CHECK: {"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0,"uri":"file://
 // CHECK: {"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid operands to binary expression ('t1' and
-// CHECK: 't1')"},"ruleId":"{{[0-9]+}}","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/
+// CHECK: 't1')"},"ruleId":"{{[A-Za-z0-9_]+}}","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/
 // CHECK: UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":
-// CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
-// CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
-// CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
-// CHECK: {"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
-// CHECK: {"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
-// CHECK: {"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
-// CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
-// CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
+// CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[A-Za-z0-9_]+}}","name":""},{"defaultConfiguration":
+// CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[A-Za-z0-9_]+}}","name":""},{"defaultConfiguration":
+// CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[A-Za-z0-9_]+}}","name":""},{"defaultConfiguration":
+// CHECK: {"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"{{[A-Za-z0-9_]+}}","name":""},{"defaultConfiguration":
+// CHECK: {"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"{{[A-Za-z0-9_]+}}","name":""},{"defaultConfiguration":
+// CHECK: {"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"{{[A-Za-z0-9_]+}}","name":""},{"defaultConfiguration":
+// CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[A-Za-z0-9_]+}}","name":""},{"defaultConfiguration":
+// CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[A-Za-z0-9_]+}}","name":""},{"defaultConfiguration":
 // CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":
-// CHECK: {"text":""},"id":"{{[0-9]+}}","name":""}],"version":"{{[0-9]+\.[0-9]+\.[0-9]+[^" ]*}}"}}}],"version":"2.1.0"}
+// CHECK: {"text":""},"id":"{{[A-Za-z0-9_]+}}","name":""}],"version":"{{[0-9]+\.[0-9]+\.[0-9]+[^" ]*}}"}}}],"version":"2.1.0"}
 // CHECK: 2 warnings and 6 errors generated.