https://github.com/dbartol updated https://github.com/llvm/llvm-project/pull/168153
>From f23857c7df24ef166aa0bb4a90e2ba13e5e49bdc Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo <[email protected]> Date: Fri, 14 Nov 2025 17:45:25 -0500 Subject: [PATCH 1/3] [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. --- clang/include/clang/Basic/DiagnosticIDs.h | 3 ++ clang/lib/Basic/DiagnosticIDs.cpp | 45 ++++++++++++++++++++++- clang/lib/Frontend/SARIFDiagnostic.cpp | 4 +- clang/test/Frontend/sarif-diagnostics.cpp | 36 +++++++++--------- 4 files changed, 68 insertions(+), 20 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticIDs.h b/clang/include/clang/Basic/DiagnosticIDs.h index 06446cf580389..9fc49325205a2 100644 --- a/clang/include/clang/Basic/DiagnosticIDs.h +++ b/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. /// diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp index a1d9d0f34d20d..e3903a3edadfd 100644 --- a/clang/lib/Basic/DiagnosticIDs.cpp +++ b/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: diff --git a/clang/lib/Frontend/SARIFDiagnostic.cpp b/clang/lib/Frontend/SARIFDiagnostic.cpp index ac27d7480de3e..ac72a7af05429 100644 --- a/clang/lib/Frontend/SARIFDiagnostic.cpp +++ b/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); diff --git a/clang/test/Frontend/sarif-diagnostics.cpp b/clang/test/Frontend/sarif-diagnostics.cpp index 767c5802ca13d..3f7adb80c67fd 100644 --- a/clang/test/Frontend/sarif-diagnostics.cpp +++ b/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. >From 402e6a80d69ee569d9399e964306598dbc9d1947 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo <[email protected]> Date: Wed, 3 Dec 2025 18:11:50 -0800 Subject: [PATCH 2/3] [clang] [diagnostics] Use `normalize_sarif` for Clang SARIF testing This change modifies the one existing Clang test for SARIF output to use the same `normalize_sarif` sed script that we use for diffing SARIF in the Clang Static Analyzer tests, rather than using a complicated set of `CHECK` statements. This did require changing the SARIF output to include line breaks. --- clang/lib/Frontend/SARIFDiagnosticPrinter.cpp | 2 +- clang/test/Analysis/lit.local.cfg | 19 +- .../sarif-diagnostics.cpp.sarif | 424 ++++++++++++++++++ clang/test/Frontend/sarif-diagnostics.cpp | 50 +-- clang/test/lit.cfg.py | 17 + 5 files changed, 448 insertions(+), 64 deletions(-) create mode 100644 clang/test/Frontend/Inputs/expected-sarif/sarif-diagnostics.cpp.sarif diff --git a/clang/lib/Frontend/SARIFDiagnosticPrinter.cpp b/clang/lib/Frontend/SARIFDiagnosticPrinter.cpp index 988159693389b..72b796f8db798 100644 --- a/clang/lib/Frontend/SARIFDiagnosticPrinter.cpp +++ b/clang/lib/Frontend/SARIFDiagnosticPrinter.cpp @@ -40,7 +40,7 @@ void SARIFDiagnosticPrinter::EndSourceFile() { assert(SARIFDiag && "SARIFDiagnostic has not been set."); Writer->endRun(); llvm::json::Value Value(Writer->createDocument()); - OS << "\n" << Value << "\n\n"; + OS << llvm::formatv("\n{0:2}\n\n", Value); OS.flush(); SARIFDiag.reset(); } diff --git a/clang/test/Analysis/lit.local.cfg b/clang/test/Analysis/lit.local.cfg index 03ab418a5a4f7..3bc2f94809c85 100644 --- a/clang/test/Analysis/lit.local.cfg +++ b/clang/test/Analysis/lit.local.cfg @@ -17,24 +17,7 @@ config.substitutions.append( ) ) -sed_cmd = "/opt/freeware/bin/sed" if "system-aix" in config.available_features else "sed" - -# Filtering command for testing SARIF output against reference output. -config.substitutions.append( - ( - "%normalize_sarif", - f"{sed_cmd} -r '%s;%s;%s;%s'" - % ( - # Replace version strings that are likely to change. - r's/"version": ".* version .*"/"version": "[clang version]"/', - r's/"version": "2.1.0"/"version": "[SARIF version]"/', - # Strip directories from file URIs - r's/"file:(\/+)([^"\/]+\/)*([^"]+)"/"file:\1[...]\/\3"/', - # Set "length" to -1 - r's/"length": [[:digit:]]+/"length": -1/' - ), - ) -) +# SARIF filtering is now shared with the rest of Clang in ../lit.cfg.py if not config.root.clang_staticanalyzer: config.unsupported = True diff --git a/clang/test/Frontend/Inputs/expected-sarif/sarif-diagnostics.cpp.sarif b/clang/test/Frontend/Inputs/expected-sarif/sarif-diagnostics.cpp.sarif new file mode 100644 index 0000000000000..21f06bf0a1b23 --- /dev/null +++ b/clang/test/Frontend/Inputs/expected-sarif/sarif-diagnostics.cpp.sarif @@ -0,0 +1,424 @@ +clang: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable] + +{ + "$schema": "https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json";, + "runs": [ + { + "artifacts": [ + { + "length": -1, + "location": { + "index": 0, + "uri": "file:///[...]/sarif-diagnostics.cpp" + }, + "mimeType": "text/plain", + "roles": [ + "resultFile" + ] + } + ], + "columnKind": "unicodeCodePoints", + "results": [ + { + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "index": 0, + "uri": "file:///[...]/sarif-diagnostics.cpp" + }, + "region": { + "endColumn": 1, + "startColumn": 1, + "startLine": 12 + } + } + } + ], + "message": { + "text": "'main' must return 'int'" + }, + "ruleId": "err_main_returns_nonint", + "ruleIndex": 0 + }, + { + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "index": 0, + "uri": "file:///[...]/sarif-diagnostics.cpp" + }, + "region": { + "endColumn": 16, + "endLine": 13, + "startColumn": 11, + "startLine": 13 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "index": 0, + "uri": "file:///[...]/sarif-diagnostics.cpp" + }, + "region": { + "endColumn": 11, + "startColumn": 11, + "startLine": 13 + } + } + } + ], + "message": { + "text": "use of undeclared identifier 'hello'" + }, + "ruleId": "err_undeclared_var_use", + "ruleIndex": 1 + }, + { + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "index": 0, + "uri": "file:///[...]/sarif-diagnostics.cpp" + }, + "region": { + "endColumn": 17, + "startColumn": 17, + "startLine": 15 + } + } + } + ], + "message": { + "text": "invalid digit 'a' in decimal constant" + }, + "ruleId": "err_invalid_digit", + "ruleIndex": 2 + }, + { + "level": "warning", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "index": 0, + "uri": "file:///[...]/sarif-diagnostics.cpp" + }, + "region": { + "endColumn": 5, + "startColumn": 5, + "startLine": 19 + } + } + } + ], + "message": { + "text": "misleading indentation; statement is not part of the previous 'if'" + }, + "ruleId": "warn_misleading_indentation", + "ruleIndex": 3 + }, + { + "level": "note", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "index": 0, + "uri": "file:///[...]/sarif-diagnostics.cpp" + }, + "region": { + "endColumn": 3, + "startColumn": 3, + "startLine": 17 + } + } + } + ], + "message": { + "text": "previous statement is here" + }, + "ruleId": "note_previous_statement", + "ruleIndex": 4 + }, + { + "level": "warning", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "index": 0, + "uri": "file:///[...]/sarif-diagnostics.cpp" + }, + "region": { + "endColumn": 13, + "endLine": 18, + "startColumn": 10, + "startLine": 18 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "index": 0, + "uri": "file:///[...]/sarif-diagnostics.cpp" + }, + "region": { + "endColumn": 10, + "startColumn": 10, + "startLine": 18 + } + } + } + ], + "message": { + "text": "unused variable 'Yes'" + }, + "ruleId": "warn_unused_variable", + "ruleIndex": 5 + }, + { + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "index": 0, + "uri": "file:///[...]/sarif-diagnostics.cpp" + }, + "region": { + "endColumn": 14, + "endLine": 21, + "startColumn": 12, + "startLine": 21 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "index": 0, + "uri": "file:///[...]/sarif-diagnostics.cpp" + }, + "region": { + "endColumn": 12, + "startColumn": 12, + "startLine": 21 + } + } + } + ], + "message": { + "text": "use of undeclared identifier 'hi'" + }, + "ruleId": "err_undeclared_var_use", + "ruleIndex": 6 + }, + { + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "index": 0, + "uri": "file:///[...]/sarif-diagnostics.cpp" + }, + "region": { + "endColumn": 1, + "startColumn": 1, + "startLine": 23 + } + } + } + ], + "message": { + "text": "extraneous closing brace ('}')" + }, + "ruleId": "err_extraneous_closing_brace", + "ruleIndex": 7 + }, + { + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "index": 0, + "uri": "file:///[...]/sarif-diagnostics.cpp" + }, + "region": { + "endColumn": 6, + "endLine": 27, + "startColumn": 5, + "startLine": 27 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "index": 0, + "uri": "file:///[...]/sarif-diagnostics.cpp" + }, + "region": { + "endColumn": 10, + "endLine": 27, + "startColumn": 9, + "startLine": 27 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "index": 0, + "uri": "file:///[...]/sarif-diagnostics.cpp" + }, + "region": { + "endColumn": 7, + "startColumn": 7, + "startLine": 27 + } + } + } + ], + "message": { + "text": "invalid operands to binary expression ('t1' and 't1')" + }, + "ruleId": "err_typecheck_invalid_operands", + "ruleIndex": 8 + } + ], + "tool": { + "driver": { + "fullName": "", + "informationUri": "https://clang.llvm.org/docs/UsersManual.html";, + "language": "en-US", + "name": "clang", + "rules": [ + { + "defaultConfiguration": { + "enabled": true, + "level": "error", + "rank": 50 + }, + "fullDescription": { + "text": "" + }, + "id": "err_main_returns_nonint", + "name": "" + }, + { + "defaultConfiguration": { + "enabled": true, + "level": "error", + "rank": 50 + }, + "fullDescription": { + "text": "" + }, + "id": "err_undeclared_var_use", + "name": "" + }, + { + "defaultConfiguration": { + "enabled": true, + "level": "error", + "rank": 50 + }, + "fullDescription": { + "text": "" + }, + "id": "err_invalid_digit", + "name": "" + }, + { + "defaultConfiguration": { + "enabled": true, + "level": "warning", + "rank": -1 + }, + "fullDescription": { + "text": "" + }, + "id": "warn_misleading_indentation", + "name": "" + }, + { + "defaultConfiguration": { + "enabled": true, + "level": "note", + "rank": -1 + }, + "fullDescription": { + "text": "" + }, + "id": "note_previous_statement", + "name": "" + }, + { + "defaultConfiguration": { + "enabled": true, + "level": "warning", + "rank": -1 + }, + "fullDescription": { + "text": "" + }, + "id": "warn_unused_variable", + "name": "" + }, + { + "defaultConfiguration": { + "enabled": true, + "level": "error", + "rank": 50 + }, + "fullDescription": { + "text": "" + }, + "id": "err_undeclared_var_use", + "name": "" + }, + { + "defaultConfiguration": { + "enabled": true, + "level": "error", + "rank": 50 + }, + "fullDescription": { + "text": "" + }, + "id": "err_extraneous_closing_brace", + "name": "" + }, + { + "defaultConfiguration": { + "enabled": true, + "level": "error", + "rank": 50 + }, + "fullDescription": { + "text": "" + }, + "id": "err_typecheck_invalid_operands", + "name": "" + } + ], + "version": "[clang version]" + } + } + } + ], + "version": "[SARIF version]" +} + +2 warnings and 6 errors generated. \ No newline at end of file diff --git a/clang/test/Frontend/sarif-diagnostics.cpp b/clang/test/Frontend/sarif-diagnostics.cpp index 3f7adb80c67fd..04cd19516fc0a 100644 --- a/clang/test/Frontend/sarif-diagnostics.cpp +++ b/clang/test/Frontend/sarif-diagnostics.cpp @@ -1,14 +1,14 @@ // RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t 2>&1 || true -// RUN: FileCheck -dump-input=always %s --input-file=%t +// RUN: cat %t | %normalize_sarif | diff -U1 -b %S/Inputs/expected-sarif/sarif-diagnostics.cpp.sarif - // FIXME: this test is incredibly fragile because the `main()` function -// must be on line 12 in order for the CHECK lines to get the correct line -// number values. +// must be on line 12 in order for the line numbers in the SARIF output +// to match the expected values // // So these comment lines are being used to ensure the code below happens // to work properly for the test coverage, which as you can imagine, is not -// the best way to structure the test. We really need to introduce a better -// tool than FileCheck for diff'ing JSON output like SARIF. +// the best way to structure the test. We should consider having a way to +// tag line numbers in the test source to match in the SARIF output. void main() { int i = hello; @@ -26,43 +26,3 @@ struct t1 { }; void f1(t1 x, t1 y) { x + y; } - -// CHECK: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable] -// CHECK: {"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length": -// Omit exact length of this file -// CHECK: ,"location":{"index":0,"uri":"file:// -// 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":"{{[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":"{{[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":"{{[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":"{{[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":"{{[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":"{{[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":"{{[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":"{{[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":"{{[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":"{{[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":"{{[A-Za-z0-9_]+}}","name":""}],"version":"{{[0-9]+\.[0-9]+\.[0-9]+[^" ]*}}"}}}],"version":"2.1.0"} -// CHECK: 2 warnings and 6 errors generated. diff --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py index 52b275c095475..58e0eb8b9597b 100644 --- a/clang/test/lit.cfg.py +++ b/clang/test/lit.cfg.py @@ -89,7 +89,24 @@ config.substitutions.append(("%PATH%", config.environment["PATH"])) +sed_cmd = "/opt/freeware/bin/sed" if "system-aix" in config.available_features else "sed" +# Filtering command for testing SARIF output against reference output. +config.substitutions.append( + ( + "%normalize_sarif", + f"{sed_cmd} -r '%s;%s;%s;%s'" + % ( + # Replace version strings that are likely to change. + r's/"version": "2.1.0"/"version": "[SARIF version]"/', + r's/"version": ".*[0-9]+\.[0-9]+\.[0-9]+.*"/"version": "[clang version]"/', + # Strip directories from file URIs + r's/"file:(\/+)([^"\/]+\/)*([^"]+)"/"file:\1[...]\/\3"/', + # Set "length" to -1 + r's/"length": [[:digit:]]+/"length": -1/' + ), + ) +) # For each occurrence of a clang tool name, replace it with the full path to # the build directory holding that tool. We explicitly specify the directories # to search to ensure that we get the tools just built and not some random >From 6ed63a1ca06d0144dc4e9c7e01ad7ef4d655b013 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo <[email protected]> Date: Thu, 4 Dec 2025 17:18:39 -0800 Subject: [PATCH 3/3] Fix formatting --- clang/test/lit.cfg.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py index 58e0eb8b9597b..8b969ef3b08c5 100644 --- a/clang/test/lit.cfg.py +++ b/clang/test/lit.cfg.py @@ -89,7 +89,9 @@ config.substitutions.append(("%PATH%", config.environment["PATH"])) -sed_cmd = "/opt/freeware/bin/sed" if "system-aix" in config.available_features else "sed" +sed_cmd = ( + "/opt/freeware/bin/sed" if "system-aix" in config.available_features else "sed" +) # Filtering command for testing SARIF output against reference output. config.substitutions.append( @@ -103,7 +105,7 @@ # Strip directories from file URIs r's/"file:(\/+)([^"\/]+\/)*([^"]+)"/"file:\1[...]\/\3"/', # Set "length" to -1 - r's/"length": [[:digit:]]+/"length": -1/' + r's/"length": [[:digit:]]+/"length": -1/', ), ) ) _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
