cjdb created this revision.
cjdb added reviewers: aaron.ballman, vaibhav.y, denik.
Herald added a project: All.
cjdb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Per §3.27.5 and §3.27.7, the `rule.id` and `ruleId` properties need to
be heirarchical names (this is only a requirement of `rule.id`, but they
need to match when both are present). We've been using the enum values
for these properties up until now, but that's neither conforming nor
stable, so we've changed it to a dotted version of the enum identifiers
(which are substantially more stable than their numeric values).
Fixes #61597.
Depends on D145201 <https://reviews.llvm.org/D145201>.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D146654
Files:
clang/include/clang/Basic/Diagnostic.h
clang/include/clang/Basic/DiagnosticIDs.h
clang/include/clang/Frontend/ASTUnit.h
clang/lib/Basic/Diagnostic.cpp
clang/lib/Basic/DiagnosticIDs.cpp
clang/lib/Frontend/ASTUnit.cpp
clang/lib/Frontend/SARIFDiagnostic.cpp
clang/test/Frontend/sarif-diagnostics.cpp
Index: clang/test/Frontend/sarif-diagnostics.cpp
===================================================================
--- clang/test/Frontend/sarif-diagnostics.cpp
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -83,7 +83,7 @@
// SARIF: "message":{
// SARIF: "text":"'main' must return 'int'"
// SARIF: },
-// SARIF: "ruleId":"3486",
+// SARIF: "ruleId":"err.main.returns.nonint",
// SARIF: "ruleIndex":0
// SARIF: },
// SARIF: {
@@ -106,7 +106,7 @@
// SARIF: "message":{
// SARIF: "text":"use of undeclared identifier 'hello'"
// SARIF: },
-// SARIF: "ruleId":"4633",
+// SARIF: "ruleId":"err.undeclared.var.use",
// SARIF: "ruleIndex":1
// SARIF: },
// SARIF: {
@@ -129,7 +129,7 @@
// SARIF: "message":{
// SARIF: "text":"invalid digit 'a' in decimal constant"
// SARIF: },
-// SARIF: "ruleId":"898",
+// SARIF: "ruleId":"err.invalid.digit",
// SARIF: "ruleIndex":2
// SARIF: },
// SARIF: {
@@ -152,7 +152,7 @@
// SARIF: "message":{
// SARIF: "text":"misleading indentation; statement is not part of the previous 'if'"
// SARIF: },
-// SARIF: "ruleId":"1826",
+// SARIF: "ruleId":"warn.misleading.indentation",
// SARIF: "ruleIndex":3
// SARIF: },
// SARIF: {
@@ -175,7 +175,7 @@
// SARIF: "message":{
// SARIF: "text":"previous statement is here"
// SARIF: },
-// SARIF: "ruleId":"1746",
+// SARIF: "ruleId":"note.previous.statement",
// SARIF: "ruleIndex":4
// SARIF: },
// SARIF: {
@@ -198,7 +198,7 @@
// SARIF: "message":{
// SARIF: "text":"unused variable 'Yes'"
// SARIF: },
-// SARIF: "ruleId":"6595",
+// SARIF: "ruleId":"warn.unused.variable",
// SARIF: "ruleIndex":5
// SARIF: },
// SARIF: {
@@ -221,7 +221,7 @@
// SARIF: "message":{
// SARIF: "text":"use of undeclared identifier 'hi'"
// SARIF: },
-// SARIF: "ruleId":"4633",
+// SARIF: "ruleId":"err.undeclared.var.use",
// SARIF: "ruleIndex":6
// SARIF: },
// SARIF: {
@@ -244,7 +244,7 @@
// SARIF: "message":{
// SARIF: "text":"extraneous closing brace ('}')"
// SARIF: },
-// SARIF: "ruleId":"1400",
+// SARIF: "ruleId":"err.extraneous.closing.brace",
// SARIF: "ruleIndex":7
// SARIF: },
// SARIF: {
@@ -295,7 +295,7 @@
// SARIF: "message":{
// SARIF: "text":"invalid operands to binary expression ('t1' and 't1')"
// SARIF: },
-// SARIF: "ruleId":"4568",
+// SARIF: "ruleId":"err.typecheck.invalid.operands",
// SARIF: "ruleIndex":8
// SARIF: },
// SARIF: {
@@ -318,7 +318,7 @@
// SARIF: "message":{
// SARIF: "text":"in file included from {{[^"]+/clang/test/Frontend/sarif-diagnostics.cpp:30:}}\n"
// SARIF: },
-// SARIF: "ruleId":"-1",
+// SARIF: "ruleId":"note.included.from",
// SARIF: "ruleIndex":9
// SARIF: },
// SARIF: {
@@ -341,7 +341,7 @@
// SARIF: "message":{
// SARIF: "text":"unknown type name 'Test'"
// SARIF: },
-// SARIF: "ruleId":"4658",
+// SARIF: "ruleId":"err.unknown.typename",
// SARIF: "ruleIndex":10
// SARIF: }
// SARIF: ],
@@ -361,7 +361,7 @@
// SARIF: "fullDescription":{
// SARIF: "text":""
// SARIF: },
-// SARIF: "id":"3486",
+// SARIF: "id":"err.main.returns.nonint",
// SARIF: "name":""
// SARIF: },
// SARIF: {
@@ -373,7 +373,7 @@
// SARIF: "fullDescription":{
// SARIF: "text":""
// SARIF: },
-// SARIF: "id":"4633",
+// SARIF: "id":"err.undeclared.var.use",
// SARIF: "name":""
// SARIF: },
// SARIF: {
@@ -385,7 +385,7 @@
// SARIF: "fullDescription":{
// SARIF: "text":""
// SARIF: },
-// SARIF: "id":"898",
+// SARIF: "id":"err.invalid.digit",
// SARIF: "name":""
// SARIF: },
// SARIF: {
@@ -397,7 +397,7 @@
// SARIF: "fullDescription":{
// SARIF: "text":""
// SARIF: },
-// SARIF: "id":"1826",
+// SARIF: "id":"warn.misleading.indentation",
// SARIF: "name":""
// SARIF: },
// SARIF: {
@@ -409,7 +409,7 @@
// SARIF: "fullDescription":{
// SARIF: "text":""
// SARIF: },
-// SARIF: "id":"1746",
+// SARIF: "id":"note.previous.statement",
// SARIF: "name":""
// SARIF: },
// SARIF: {
@@ -421,7 +421,7 @@
// SARIF: "fullDescription":{
// SARIF: "text":""
// SARIF: },
-// SARIF: "id":"6595",
+// SARIF: "id":"warn.unused.variable",
// SARIF: "name":""
// SARIF: },
// SARIF: {
@@ -433,7 +433,7 @@
// SARIF: "fullDescription":{
// SARIF: "text":""
// SARIF: },
-// SARIF: "id":"4633",
+// SARIF: "id":"err.undeclared.var.use",
// SARIF: "name":""
// SARIF: },
// SARIF: {
@@ -445,7 +445,7 @@
// SARIF: "fullDescription":{
// SARIF: "text":""
// SARIF: },
-// SARIF: "id":"1400",
+// SARIF: "id":"err.extraneous.closing.brace",
// SARIF: "name":""
// SARIF: },
// SARIF: {
@@ -457,7 +457,7 @@
// SARIF: "fullDescription":{
// SARIF: "text":""
// SARIF: },
-// SARIF: "id":"4568",
+// SARIF: "id":"err.typecheck.invalid.operands",
// SARIF: "name":""
// SARIF: },
// SARIF: {
@@ -469,7 +469,7 @@
// SARIF: "fullDescription":{
// SARIF: "text":""
// SARIF: },
-// SARIF: "id":"-1",
+// SARIF: "id":"note.included.from"
// SARIF: "name":""
// SARIF: },
// SARIF: {
@@ -481,7 +481,7 @@
// SARIF: "fullDescription":{
// SARIF: "text":""
// SARIF: },
-// SARIF: "id":"4658",
+// SARIF: "id":"err.unknown.typename",
// SARIF: "name":""
// SARIF: }
// SARIF: ],
Index: clang/lib/Frontend/SARIFDiagnostic.cpp
===================================================================
--- clang/lib/Frontend/SARIFDiagnostic.cpp
+++ clang/lib/Frontend/SARIFDiagnostic.cpp
@@ -47,7 +47,10 @@
if (!Diag)
return;
- SarifRule Rule = SarifRule::create().setRuleId(std::to_string(Diag->getID()));
+ std::string StableName =
+ Diag->getDiags()->getDiagnosticIDs()->getStableName(Diag->getID()).str();
+ std::replace(StableName.begin(), StableName.end(), '_', '.');
+ SarifRule Rule = SarifRule::create().setRuleId(StableName);
Rule = addDiagnosticLevelToRule(Rule, Level);
@@ -211,8 +214,7 @@
}
void SARIFDiagnostic::emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc) {
- SarifRule Rule = SarifRule::create().setRuleId(
- "-1"); // FIXME(llvm-project#61597) replace with something interesting
+ SarifRule Rule = SarifRule::create().setRuleId("note.included.from");
Rule = addDiagnosticLevelToRule(Rule, DiagnosticsEngine::Level::Note);
unsigned RuleIdx = Writer->createRule(Rule);
Index: clang/lib/Frontend/ASTUnit.cpp
===================================================================
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -2417,8 +2417,8 @@
FH.RemoveRange = CharSourceRange::getCharRange(BL, EL);
}
- Result.push_back(StoredDiagnostic(SD.Level, SD.ID,
- SD.Message, Loc, Ranges, FixIts));
+ Result.push_back(StoredDiagnostic(SD.Level, SD.ID, SD.Message,
+ SD.StableName, Loc, Ranges, FixIts));
}
Result.swap(Out);
}
Index: clang/lib/Basic/DiagnosticIDs.cpp
===================================================================
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -99,6 +99,28 @@
#undef DIAG
};
+// Stable names
+const char *const StaticDiagInfoStableNames[] = {
+#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \
+ SHOWINSYSHEADER, SHOWINSYSMACRO, DEFERRABLE, CATEGORY) \
+ #ENUM,
+// clang-format off
+#include "clang/Basic/DiagnosticCommonKinds.inc"
+#include "clang/Basic/DiagnosticDriverKinds.inc"
+#include "clang/Basic/DiagnosticFrontendKinds.inc"
+#include "clang/Basic/DiagnosticSerializationKinds.inc"
+#include "clang/Basic/DiagnosticLexKinds.inc"
+#include "clang/Basic/DiagnosticParseKinds.inc"
+#include "clang/Basic/DiagnosticASTKinds.inc"
+#include "clang/Basic/DiagnosticCommentKinds.inc"
+#include "clang/Basic/DiagnosticCrossTUKinds.inc"
+#include "clang/Basic/DiagnosticSemaKinds.inc"
+#include "clang/Basic/DiagnosticAnalysisKinds.inc"
+#include "clang/Basic/DiagnosticRefactoringKinds.inc"
+// clang-format on
+#undef DIAG
+};
+
// Diagnostic classes.
enum {
CLASS_NOTE = 0x01,
@@ -134,6 +156,11 @@
return StringRef(&Table[StringOffset], DescriptionLen);
}
+ StringRef getStableName() const {
+ size_t MyIndex = this - &StaticDiagInfo[0];
+ return StringRef(StaticDiagInfoStableNames[MyIndex]);
+ }
+
diag::Flavor getFlavor() const {
return Class == CLASS_REMARK ? diag::Flavor::Remark
: diag::Flavor::WarningOrError;
@@ -373,6 +400,12 @@
return DiagInfo[DiagID-DIAG_UPPER_LIMIT].second;
}
+ StringRef getStableName(unsigned DiagID) const {
+ assert(DiagID - DIAG_UPPER_LIMIT < DiagInfo.size() &&
+ "Invalid diagnostic ID");
+ return DiagInfo[DiagID - DIAG_UPPER_LIMIT].second;
+ }
+
/// getLevel - Return the level of the specified custom diagnostic.
DiagnosticIDs::Level getLevel(unsigned DiagID) const {
assert(DiagID - DIAG_UPPER_LIMIT < DiagInfo.size() &&
@@ -469,6 +502,13 @@
return CustomDiagInfo->getDescription(DiagID);
}
+StringRef DiagnosticIDs::getStableName(unsigned DiagID) const {
+ if (const StaticDiagInfoRec *Info = GetDiagInfo(DiagID))
+ return Info->getStableName();
+ assert(CustomDiagInfo && "Invalid CustomDiagInfo");
+ return CustomDiagInfo->getStableName(DiagID);
+}
+
static DiagnosticIDs::Level toLevel(diag::Severity SV) {
switch (SV) {
case diag::Severity::Ignored:
Index: clang/lib/Basic/Diagnostic.cpp
===================================================================
--- clang/lib/Basic/Diagnostic.cpp
+++ clang/lib/Basic/Diagnostic.cpp
@@ -1154,8 +1154,8 @@
}
StoredDiagnostic::StoredDiagnostic(DiagnosticsEngine::Level Level, unsigned ID,
- StringRef Message)
- : ID(ID), Level(Level), Message(Message) {}
+ StringRef Message, StringRef StableName)
+ : ID(ID), Level(Level), Message(Message), StableName(StableName) {}
StoredDiagnostic::StoredDiagnostic(DiagnosticsEngine::Level Level,
const Diagnostic &Info)
@@ -1172,13 +1172,13 @@
}
StoredDiagnostic::StoredDiagnostic(DiagnosticsEngine::Level Level, unsigned ID,
- StringRef Message, FullSourceLoc Loc,
+ StringRef Message, StringRef StableName,
+ FullSourceLoc Loc,
ArrayRef<CharSourceRange> Ranges,
ArrayRef<FixItHint> FixIts)
- : ID(ID), Level(Level), Loc(Loc), Message(Message),
- Ranges(Ranges.begin(), Ranges.end()), FixIts(FixIts.begin(), FixIts.end())
-{
-}
+ : ID(ID), Level(Level), Loc(Loc), Message(Message), StableName(StableName),
+ Ranges(Ranges.begin(), Ranges.end()),
+ FixIts(FixIts.begin(), FixIts.end()) {}
llvm::raw_ostream &clang::operator<<(llvm::raw_ostream &OS,
const StoredDiagnostic &SD) {
Index: clang/include/clang/Frontend/ASTUnit.h
===================================================================
--- clang/include/clang/Frontend/ASTUnit.h
+++ clang/include/clang/Frontend/ASTUnit.h
@@ -99,6 +99,7 @@
DiagnosticsEngine::Level Level;
std::string Message;
std::string Filename;
+ std::string StableName;
unsigned LocOffset;
std::vector<std::pair<unsigned, unsigned>> Ranges;
std::vector<StandaloneFixIt> FixIts;
Index: clang/include/clang/Basic/DiagnosticIDs.h
===================================================================
--- clang/include/clang/Basic/DiagnosticIDs.h
+++ clang/include/clang/Basic/DiagnosticIDs.h
@@ -197,6 +197,9 @@
/// Given a diagnostic ID, return a description of the issue.
StringRef getDescription(unsigned DiagID) const;
+ /// Given a diagnostic ID, return its stable name
+ StringRef getStableName(unsigned DiagID) const;
+
/// Return true if the unmapped diagnostic levelof the specified
/// diagnostic ID is a Warning or Extension.
///
Index: clang/include/clang/Basic/Diagnostic.h
===================================================================
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -1692,6 +1692,7 @@
DiagnosticsEngine::Level Level;
FullSourceLoc Loc;
std::string Message;
+ std::string StableName;
std::vector<CharSourceRange> Ranges;
std::vector<FixItHint> FixIts;
@@ -1699,9 +1700,9 @@
StoredDiagnostic() = default;
StoredDiagnostic(DiagnosticsEngine::Level Level, const Diagnostic &Info);
StoredDiagnostic(DiagnosticsEngine::Level Level, unsigned ID,
- StringRef Message);
+ StringRef Message, StringRef StableName);
StoredDiagnostic(DiagnosticsEngine::Level Level, unsigned ID,
- StringRef Message, FullSourceLoc Loc,
+ StringRef Message, StringRef StableName, FullSourceLoc Loc,
ArrayRef<CharSourceRange> Ranges,
ArrayRef<FixItHint> Fixits);
@@ -1712,6 +1713,7 @@
DiagnosticsEngine::Level getLevel() const { return Level; }
const FullSourceLoc &getLocation() const { return Loc; }
StringRef getMessage() const { return Message; }
+ StringRef getStableName() const { return StableName; }
void setLocation(FullSourceLoc Loc) { this->Loc = Loc; }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
