https://github.com/anonymouspc created
https://github.com/llvm/llvm-project/pull/174106
# New feature
This PR introduces **nested/indented** diagnostic message when calling with
`clang++ -fdiagnostics-format=sarif`.
In general, the user can now see a nested tree-like diagnositc, where one
`warning`/`error` mounts several `notes` (and might recursively mount more
`notes`, which seems like a tree). User may even fold the diagnostics through
some sarif viewer.
In detail, in the `.sarif` output, `runs.results.relatedLocations` originally
only represents some `In file included from...` information, but now can
`variantly` represents child notes with diagnostics.
*(Note that now the `.sarif` format of `clang++ -fdiagnostics-format=sarif` is
same to `g++ -fdiagnostics-format=sarif`).*
# Changes to codes
### `clang/include/clang/Frontend/SARIFDiagnostic.h`,
`clang/lib/Frontend/SARIFDiagnostic.cpp`
- Logically define sarif-tree.
- We know that the `clang++` diagnostics is **logically** a tree, which means
that one error/warning often corresponds to several notes (and maybe
recursively more notes). Let's take an example, a typical `clang++` diagnostics
might looks like:
```
error: invalid binary operand '+' to A and B.
note: candidate 1
note: why candidate 1 is invalid
note: candidate 2
note: why candidate 2 is invalid
error: static assertion failed.
note: in instantiation of C
note: in instantiation of D
note: concept/constraints not satisfied.
```
- Here we rewrites the logical-data-structure of sarif into a tree (see
`SARIFDiagnostic::Node`), that is:
- when the `DiagnosticRenderer` emits diagnostics, `SARIFDiagnostic`
organize them into a recursive tree;
- when the `SARIFDiagnostic` destructs, it flush them all into the
`SarifDocumentWriter`.
### `clang/include/clang/Basic/Sarif.h`, `clang/lib/Basic/Sarif.cpp`
- Physically define sarif-tree.
- **Physically**, the sarif diagnostics format has a plain structure, that is:
- one `result` may carry several `Location`s and several
`relatedLocation`s
- a `Location` is the place where error occurs, e.g. `main.cpp:3:4`,
- a `relatedLocation` is either a
- `In file included from...` location, or a
- *`child-result`*, who takes message, location, and **nesting**.
- which **logically** corresponds a `clang++` note diagnostic.
- Here, we expand the definition of `SarifResult::RelatedLocations` from
`vector<CharSourceRange>` into `vector<variant<SarifChildResult,
CharSourceRange>>`, and, at the same time update the corresponding
serialization function for `relatedLocations`.
- *(Note that GCC also makes sarif output in the same way: put child `results`
into `relatedLocations` and then mark them with `properties.nestingLevel`)*.
### `clang/lib/Frontend/SARIFDiagnosticPrinter.cpp`
- Adjust the printer logic.
- Now the `SARIFDiagnotic` flushes all the diagnostics (at one time) when it
destructs, so we need to adjust the sequence of `SARIFDiagnosticPrinter` in
order that the `Writer->endRun()` happens **after** the `SARIFDiagnostic`
flushes.
### `clang/include/clang/Basic/DiagnosticDriverKinds.td`,
`clang/lib/Driver/ToolChains/Clang.cpp`
- Disable sarif warnings.
- Currently the `-Wsarif-format-unstable` warning happens **before** the
`SARIFDiagnostic` works, This warning message will not appear in the `json`
output, instead, it triggers in text-mode before the json output. To make the
`clang++` output machine-parsable, let's remove this warning (or move this
warning into the json).
- In my perspective, after many efforts, now the sarif mode seems ok to be
used. Let's remove the `-Wsarif-format-unstable` warning.
### `clang/unittests/Basic/SarifTest.cpp`
- Update test.
- Update these corresponding test for `SARIFDiagnostic` and the
`relatedLocations` (with child results).
>From 81d24d25ed32ef5ec605049c1ae7bb9cae65b2b0 Mon Sep 17 00:00:00 2001
From: anonymous <[email protected]>
Date: Thu, 1 Jan 2026 02:00:40 +0800
Subject: [PATCH 1/5] Logically define sarif-tree.
We know that the `clang++` diagnostics is **logically** a tree, which means
that one error/warning often corresponds to several notes (and maybe
recursively more notes). Let's take an example, a typical `clang++` diagnostics
might looks like:
```
error: invalid binary operand '+' to A and B.
note: candidate 1
note: why candidate 1 is invalid
note: candidate 2
note: why candidate 2 is invalid
error: static assertion failed.
note: in instantiation of C
note: in instantiation of D
note: concept/constraints not satisfied.
```
This commit rewrites the logical-data-structure of sarif into a tree (see
`SARIFDiagnostic::Node`), that is:
- when the `DiagnosticRenderer` emits diagnostics, `SARIFDiagnostic` organize
them into a recursive tree;
- when the `SARIFDiagnostic` destructs, it flush them all into the
`SarifDocumentWriter`.
---
.../include/clang/Frontend/SARIFDiagnostic.h | 89 +++--
clang/lib/Frontend/SARIFDiagnostic.cpp | 314 +++++++++++-------
2 files changed, 258 insertions(+), 145 deletions(-)
diff --git a/clang/include/clang/Frontend/SARIFDiagnostic.h
b/clang/include/clang/Frontend/SARIFDiagnostic.h
index 7a6f27eb3b9fa..70b5c80b68812 100644
--- a/clang/include/clang/Frontend/SARIFDiagnostic.h
+++ b/clang/include/clang/Frontend/SARIFDiagnostic.h
@@ -14,9 +14,13 @@
#ifndef LLVM_CLANG_FRONTEND_SARIFDIAGNOSTIC_H
#define LLVM_CLANG_FRONTEND_SARIFDIAGNOSTIC_H
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/LangOptions.h"
#include "clang/Basic/Sarif.h"
+#include "clang/Basic/SourceLocation.h"
#include "clang/Frontend/DiagnosticRenderer.h"
-#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/SmallVector.h"
namespace clang {
@@ -25,7 +29,7 @@ class SARIFDiagnostic : public DiagnosticRenderer {
SARIFDiagnostic(raw_ostream &OS, const LangOptions &LangOpts,
DiagnosticOptions &DiagOpts, SarifDocumentWriter *Writer);
- ~SARIFDiagnostic() = default;
+ ~SARIFDiagnostic();
SARIFDiagnostic &operator=(const SARIFDiagnostic &&) = delete;
SARIFDiagnostic(SARIFDiagnostic &&) = delete;
@@ -36,7 +40,7 @@ class SARIFDiagnostic : public DiagnosticRenderer {
void emitDiagnosticMessage(FullSourceLoc Loc, PresumedLoc PLoc,
DiagnosticsEngine::Level Level, StringRef Message,
ArrayRef<CharSourceRange> Ranges,
- DiagOrStoredDiag D) override;
+ DiagOrStoredDiag Diag) override;
void emitDiagnosticLoc(FullSourceLoc Loc, PresumedLoc PLoc,
DiagnosticsEngine::Level Level,
@@ -55,28 +59,63 @@ class SARIFDiagnostic : public DiagnosticRenderer {
StringRef ModuleName) override;
private:
- // Shared between SARIFDiagnosticPrinter and this renderer.
- SarifDocumentWriter *Writer;
-
- SarifResult addLocationToResult(SarifResult Result, FullSourceLoc Loc,
- PresumedLoc PLoc,
- ArrayRef<CharSourceRange> Ranges,
- const Diagnostic &Diag);
-
- SarifResult addRelatedLocationToResult(SarifResult Result, FullSourceLoc Loc,
- PresumedLoc PLoc);
-
- llvm::SmallVector<CharSourceRange>
- getSarifLocation(FullSourceLoc Loc, PresumedLoc PLoc,
- ArrayRef<CharSourceRange> Ranges);
-
- SarifRule addDiagnosticLevelToRule(SarifRule Rule,
- DiagnosticsEngine::Level Level);
-
- llvm::StringRef emitFilename(StringRef Filename, const SourceManager &SM);
-
- llvm::SmallVector<std::pair<FullSourceLoc, PresumedLoc>>
- RelatedLocationsCache;
+ class Node {
+ public:
+ // Subclasses
+ struct Result {
+ DiagnosticsEngine::Level Level;
+ std::string Message;
+ DiagOrStoredDiag Diag;
+ };
+
+ struct Option {
+ const LangOptions* LangOptsPtr;
+ const DiagnosticOptions* DiagnosticOptsPtr;
+ };
+
+ struct Location {
+ FullSourceLoc Loc;
+ PresumedLoc PLoc;
+ llvm::SmallVector<CharSourceRange> Ranges;
+
+ // Methods to construct a llvm-style location.
+ llvm::SmallVector<CharSourceRange>
getCharSourceRangesWithOption(Option);
+ };
+
+ // Constructor
+ Node(Result Result_, Option Option_, int Nesting);
+
+ // Operations on building a node-tree.
+ // Arguments and results are all in node-style.
+ Node& getParent();
+ Node& getForkableParent();
+ llvm::SmallVector<std::unique_ptr<Node>>& getChildrenPtrs();
+ Node& addChildResult(Result);
+ Node& addLocation(Location);
+ Node& addRelatedLocation(Location);
+
+ // Methods to access underlying data for other llvm-components to read
from it.
+ // Arguments and results are all in llvm-style.
+ unsigned getDiagID();
+ DiagnosticsEngine::Level getLevel();
+ std::string getDiagnosticMessage();
+ llvm::SmallVector<CharSourceRange> getLocations();
+ llvm::SmallVector<CharSourceRange> getRelatedLocations();
+ int getNesting();
+
+ private:
+ Result Result_;
+ llvm::SmallVector<Location> Locations;
+ llvm::SmallVector<Location> RelatedLocations;
+ Option Option_;
+ int Nesting;
+ Node* ParentPtr = nullptr;
+ llvm::SmallVector<std::unique_ptr<Node>> ChildrenPtrs = {};
+ };
+
+ Node Root;
+ Node* Current = &Root;
+ SarifDocumentWriter *Writer; // Shared between SARIFDiagnosticPrinter and
this renderer.
};
} // end namespace clang
diff --git a/clang/lib/Frontend/SARIFDiagnostic.cpp
b/clang/lib/Frontend/SARIFDiagnostic.cpp
index 2cd32ce97ea85..930d1b1383ddb 100644
--- a/clang/lib/Frontend/SARIFDiagnostic.cpp
+++ b/clang/lib/Frontend/SARIFDiagnostic.cpp
@@ -7,108 +7,210 @@
//===----------------------------------------------------------------------===//
#include "clang/Frontend/SARIFDiagnostic.h"
-#include "clang/Basic/CharInfo.h"
+#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticOptions.h"
-#include "clang/Basic/FileManager.h"
+#include "clang/Basic/DiagnosticSema.h"
#include "clang/Basic/Sarif.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/Lexer.h"
#include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/SmallString.h"
-#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/ErrorHandling.h"
-#include "llvm/Support/ErrorOr.h"
#include "llvm/Support/Locale.h"
-#include "llvm/Support/Path.h"
-#include "llvm/Support/raw_ostream.h"
#include <algorithm>
#include <string>
namespace clang {
+// In sarif mode,
+// a diagnostics 'group' have 1 top-level error/warning and several sub-level
notes.
+// For example:
+//
+// error: static assertion failed.
+// note: in instantiation of 'cat::meow'.
+// note: because concept 'paper_tiger' would be invalid.
+// error: invalid operands to binary expression 'cat::meow' and 'dog::wolf'.
+// note: candidate function not viable.
+// note: no known conversion from 'tiger::meooooow' to 'cat::meow'
+// note: candidate function ignored.
+// note: constraints not satisfied.
+// note: ... (candidates)
+// note: ... (reasons)
+// note: too many candidates.
+// error: too many errors occured, stopping now.
+
SARIFDiagnostic::SARIFDiagnostic(raw_ostream &OS, const LangOptions &LangOpts,
DiagnosticOptions &DiagOpts,
SarifDocumentWriter *Writer)
- : DiagnosticRenderer(LangOpts, DiagOpts), Writer(Writer) {}
+ : DiagnosticRenderer(LangOpts, DiagOpts),
+ Root(Node::Result(), Node::Option{&LangOpts, &DiagOpts},
/*Nesting=*/-1), // The root does not represents a diagnostic.
+ Current(&Root),
+ Writer(Writer)
+{
+ // Don't print 'X warnings and Y errors generated'.
+ DiagOpts.ShowCarets = false;
+}
+
+// helper function
+namespace {
+ template <class NodeType, class IterateFuncType, class ApplyFuncType>
+ void RecursiveFor (NodeType&& Node, IterateFuncType&& IterateFunc,
ApplyFuncType&& ApplyFunc) {
+ for (auto&& Child : IterateFunc(Node)) {
+ ApplyFunc(*Child);
+ RecursiveFor(*Child, IterateFunc, ApplyFunc);
+ }
+ }
+} // namespace
+
+SARIFDiagnostic::~SARIFDiagnostic() {
+ // clang-format off
+ for (auto& TopLevelDiagnosticsPtr : Root.getChildrenPtrs()) { // For each
top-level error/warnings.
+ unsigned DiagID = TopLevelDiagnosticsPtr->getDiagID();
+ SarifRule Rule = SarifRule::create() // Each top-level error/warning has a
corresponding Rule.
+ .setRuleId(std::to_string(DiagID))
+ .setDefaultConfiguration(
+ SarifReportingConfiguration::create()
+ .setLevel(
+ TopLevelDiagnosticsPtr->getLevel() ==
DiagnosticsEngine::Level::Note ? SarifResultLevel::Note :
+ TopLevelDiagnosticsPtr->getLevel() ==
DiagnosticsEngine::Level::Remark ? SarifResultLevel::Note :
+ TopLevelDiagnosticsPtr->getLevel() ==
DiagnosticsEngine::Level::Warning ? SarifResultLevel::Warning :
+ TopLevelDiagnosticsPtr->getLevel() ==
DiagnosticsEngine::Level::Error ? SarifResultLevel::Error :
+ TopLevelDiagnosticsPtr->getLevel() ==
DiagnosticsEngine::Level::Fatal ? SarifResultLevel::Error :
+ (assert(false && "Invalid diagnostic
type"), SarifResultLevel::None)
+ )
+ .setRank(
+ TopLevelDiagnosticsPtr->getLevel() <=
DiagnosticsEngine::Level::Warning ? 0 :
+ TopLevelDiagnosticsPtr->getLevel() ==
DiagnosticsEngine::Level::Error ? 50 :
+ TopLevelDiagnosticsPtr->getLevel() ==
DiagnosticsEngine::Level::Fatal ? 100 :
+ (assert(false && "Invalid diagnostic
type"), 0)
+ )
+ );
+ unsigned RuleIndex = Writer->createRule(Rule); // Write into Writer.
+
+ SarifResult Result = SarifResult::create(RuleIndex)
+ .setDiagnosticMessage(TopLevelDiagnosticsPtr->getDiagnosticMessage())
+ .addLocations(TopLevelDiagnosticsPtr->getLocations())
+ .addRelatedLocations(TopLevelDiagnosticsPtr->getRelatedLocations());
+ RecursiveFor(*TopLevelDiagnosticsPtr, [] (Node& Node) -> auto& { return
Node.getChildrenPtrs(); }, [&] (Node& Node) { // For each (recursive)
ChildResults.
+ Result.addRelatedLocations({
+ SarifChildResult::create()
+ .setDiagnosticMessage(Node.getDiagnosticMessage())
+ .addLocations(Node.getLocations())
+ .setNesting(Node.getNesting())
+ });
+ Result.addRelatedLocations(Node.getRelatedLocations());
+ });
+ Writer->appendResult(Result); // Write into Writer
+ }
+ // clang-format on
+}
-// FIXME(llvm-project/issues/57323): Refactor Diagnostic classes.
void SARIFDiagnostic::emitDiagnosticMessage(
FullSourceLoc Loc, PresumedLoc PLoc, DiagnosticsEngine::Level Level,
StringRef Message, ArrayRef<clang::CharSourceRange> Ranges,
- DiagOrStoredDiag D) {
+ DiagOrStoredDiag Diag) {
- const auto *Diag = D.dyn_cast<const Diagnostic *>();
+ if (Level >= DiagnosticsEngine::Level::Warning) {
+ Current = &Root; // If this is a top-level error/warning, repoint Current
to Root.
+ } else {
+ if (Message.starts_with("candidate"))
+ Current = &Current->getForkableParent(); // If this is an forked-case
note, repoint Current to the nearest forkable Node.
+ }
+ Current = &Current->addChildResult(Node::Result{Level, std::string(Message),
Diag}); // add child to the parent error/warning/note Node.
+ Current = &Current->addLocation(Node::Location{Loc, PLoc,
llvm::SmallVector<CharSourceRange>(Ranges)});
+}
- if (!Diag)
- return;
+void SARIFDiagnostic::emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc)
{
+ Current = &Current->addRelatedLocation(Node::Location{Loc, PLoc, {}});
+}
- SarifRule Rule =
SarifRule::create().setRuleId(std::to_string(Diag->getID()));
+void SARIFDiagnostic::emitImportLocation(FullSourceLoc Loc, PresumedLoc PLoc,
StringRef ModuleName) {
+ Current = &Current->addRelatedLocation(Node::Location{Loc, PLoc, {}});
+}
- Rule = addDiagnosticLevelToRule(Rule, Level);
+SARIFDiagnostic::Node::Node(Result Result_, Option Option_, int Nesting)
+ : Result_(std::move(Result_)), Option_(std::move(Option_)), Nesting(Nesting)
{}
- unsigned RuleIdx = Writer->createRule(Rule);
+SARIFDiagnostic::Node& SARIFDiagnostic::Node::getParent() {
+ assert(ParentPtr && "getParent() of SARIFDiagnostic::Root!");
+ return *ParentPtr;
+}
- SarifResult Result =
- SarifResult::create(RuleIdx).setDiagnosticMessage(Message);
+SARIFDiagnostic::Node& SARIFDiagnostic::Node::getForkableParent() {
+ Node* Ptr = this;
+ while (Ptr->getLevel() <= DiagnosticsEngine::Note) // The forkable node here
"is and only is" warning/error/fatal.
+ Ptr = &Ptr->getParent();
+ return *Ptr;
+}
- if (Loc.isValid())
- Result = addLocationToResult(Result, Loc, PLoc, Ranges, *Diag);
+llvm::SmallVector<std::unique_ptr<SARIFDiagnostic::Node>>&
SARIFDiagnostic::Node::getChildrenPtrs() {
+ return ChildrenPtrs;
+}
- for (auto &[RelLoc, RelPLoc] : RelatedLocationsCache)
- Result = addRelatedLocationToResult(Result, RelLoc, RelPLoc);
- RelatedLocationsCache.clear();
+SARIFDiagnostic::Node& SARIFDiagnostic::Node::addChildResult(Result
ChildResult) {
+
ChildrenPtrs.push_back(std::make_unique<Node>(Node::Result(std::move(ChildResult)),
Node::Option(std::move(Option_)), Nesting + 1));
+ ChildrenPtrs.back()->ParentPtr = this; // I am the parent of this new child.
+ return *ChildrenPtrs.back();
+}
- Writer->appendResult(Result);
+SARIFDiagnostic::Node& SARIFDiagnostic::Node::addLocation(Location Location) {
+ Locations.push_back(std::move(Location));
+ return *this;
}
-void SARIFDiagnostic::emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc)
{
- // We always emit include location before results, for example:
- //
- // In file included from ...
- // In file included from ...
- // error: ...
- //
- // At this time We cannot peek the SarifRule. But what we
- // do is to push it into a cache and wait for next time
- // \ref SARIFDiagnostic::emitDiagnosticMessage to pick it up.
- RelatedLocationsCache.push_back({Loc, PLoc});
+SARIFDiagnostic::Node& SARIFDiagnostic::Node::addRelatedLocation(Location
Location) {
+ RelatedLocations.push_back(std::move(Location));
+ return *this;
}
-void SARIFDiagnostic::emitImportLocation(FullSourceLoc Loc, PresumedLoc PLoc,
- StringRef ModuleName) {
- RelatedLocationsCache.push_back({Loc, PLoc});
+unsigned SARIFDiagnostic::Node::getDiagID() {
+ return llvm::isa<const Diagnostic*>(Result_.Diag) ?
+ Result_.Diag.dyn_cast<const Diagnostic*>()->getID() :
+ Result_.Diag.dyn_cast<const StoredDiagnostic*>()->getID();
}
-SarifResult SARIFDiagnostic::addLocationToResult(
- SarifResult Result, FullSourceLoc Loc, PresumedLoc PLoc,
- ArrayRef<CharSourceRange> Ranges, const Diagnostic &Diag) {
- auto Locations = getSarifLocation(Loc, PLoc, Ranges);
- return Result.addLocations(Locations);
+DiagnosticsEngine::Level SARIFDiagnostic::Node::getLevel() {
+ return Result_.Level;
}
-SarifResult SARIFDiagnostic::addRelatedLocationToResult(SarifResult Result,
- FullSourceLoc Loc,
- PresumedLoc PLoc) {
- auto Locations = getSarifLocation(Loc, PLoc, {});
- return Result.addRelatedLocations(Locations);
+std::string SARIFDiagnostic::Node::getDiagnosticMessage() {
+ return Result_.Message;
}
-llvm::SmallVector<CharSourceRange>
-SARIFDiagnostic::getSarifLocation(FullSourceLoc Loc, PresumedLoc PLoc,
- ArrayRef<CharSourceRange> Ranges) {
- SmallVector<CharSourceRange> Locations = {};
+llvm::SmallVector<CharSourceRange> SARIFDiagnostic::Node::getLocations() {
+ llvm::SmallVector<CharSourceRange> CharSourceRanges;
+ std::for_each(Locations.begin(), Locations.end(), [&] (Location& Location) {
+ CharSourceRanges.append(Location.getCharSourceRangesWithOption(Option_));
+ });
+ return CharSourceRanges;
+}
+
+llvm::SmallVector<CharSourceRange>
SARIFDiagnostic::Node::getRelatedLocations() {
+ llvm::SmallVector<CharSourceRange> CharSourceRanges;
+ std::for_each(RelatedLocations.begin(), RelatedLocations.end(), [&]
(Location& RelatedLocation) {
+
CharSourceRanges.append(RelatedLocation.getCharSourceRangesWithOption(Option_));
+ });
+ return CharSourceRanges;
+}
+
+int SARIFDiagnostic::Node::getNesting() {
+ return Nesting;
+}
+llvm::SmallVector<CharSourceRange>
SARIFDiagnostic::Node::Location::getCharSourceRangesWithOption(Option Option) {
+ SmallVector<CharSourceRange> Locations = {};
+
if (PLoc.isInvalid()) {
+ // FIXME(llvm-project/issues/57366): File-only locations
// At least add the file name if available:
FileID FID = Loc.getFileID();
if (FID.isValid()) {
if (OptionalFileEntryRef FE = Loc.getFileEntryRef()) {
- emitFilename(FE->getName(), Loc.getManager());
- // FIXME(llvm-project/issues/57366): File-only locations
+ // EmitFilename(FE->getName(), Loc.getManager());
}
}
return {};
@@ -135,11 +237,11 @@ SARIFDiagnostic::getSarifLocation(FullSourceLoc Loc,
PresumedLoc PLoc,
if (BInfo.first != CaretFileID || EInfo.first != CaretFileID)
continue;
- // Add in the length of the token, so that we cover multi-char
+ // add in the length of the token, so that we cover multi-char
// tokens.
unsigned TokSize = 0;
if (IsTokenRange)
- TokSize = Lexer::MeasureTokenLength(E, SM, LangOpts);
+ TokSize = Lexer::MeasureTokenLength(E, SM, *Option.LangOptsPtr);
FullSourceLoc BF(B, SM), EF(E, SM);
SourceLocation BeginLoc = SM.translateLineCol(
@@ -149,15 +251,15 @@ SARIFDiagnostic::getSarifLocation(FullSourceLoc Loc,
PresumedLoc PLoc,
Locations.push_back(
CharSourceRange{SourceRange{BeginLoc, EndLoc}, /* ITR = */ false});
- // FIXME: Additional ranges should use presumed location in both
+ // FIXME: additional ranges should use presumed location in both
// Text and SARIF diagnostics.
}
auto &SM = Loc.getManager();
auto FID = PLoc.getFileID();
// Visual Studio 2010 or earlier expects column number to be off by one.
- unsigned int ColNo = (LangOpts.MSCompatibilityVersion &&
- !LangOpts.isCompatibleWithMSVC(LangOptions::MSVC2012))
+ unsigned int ColNo = (Option.LangOptsPtr->MSCompatibilityVersion &&
+
!Option.LangOptsPtr->isCompatibleWithMSVC(LangOptions::MSVC2012))
? PLoc.getColumn() - 1
: PLoc.getColumn();
SourceLocation DiagLoc = SM.translateLineCol(FID, PLoc.getLine(), ColNo);
@@ -170,67 +272,39 @@ SARIFDiagnostic::getSarifLocation(FullSourceLoc Loc,
PresumedLoc PLoc,
return Locations;
}
-SarifRule
-SARIFDiagnostic::addDiagnosticLevelToRule(SarifRule Rule,
- DiagnosticsEngine::Level Level) {
- auto Config = SarifReportingConfiguration::create();
-
- switch (Level) {
- case DiagnosticsEngine::Note:
- Config = Config.setLevel(SarifResultLevel::Note);
- break;
- case DiagnosticsEngine::Remark:
- Config = Config.setLevel(SarifResultLevel::None);
- break;
- case DiagnosticsEngine::Warning:
- Config = Config.setLevel(SarifResultLevel::Warning);
- break;
- case DiagnosticsEngine::Error:
- Config = Config.setLevel(SarifResultLevel::Error).setRank(50);
- break;
- case DiagnosticsEngine::Fatal:
- Config = Config.setLevel(SarifResultLevel::Error).setRank(100);
- break;
- case DiagnosticsEngine::Ignored:
- assert(false && "Invalid diagnostic type");
- }
-
- return Rule.setDefaultConfiguration(Config);
-}
-
-llvm::StringRef SARIFDiagnostic::emitFilename(StringRef Filename,
- const SourceManager &SM) {
- if (DiagOpts.AbsolutePath) {
- auto File = SM.getFileManager().getOptionalFileRef(Filename);
- if (File) {
- // We want to print a simplified absolute path, i. e. without "dots".
- //
- // The hardest part here are the paths like "<part1>/<link>/../<part2>".
- // On Unix-like systems, we cannot just collapse "<link>/..", because
- // paths are resolved sequentially, and, thereby, the path
- // "<part1>/<part2>" may point to a different location. That is why
- // we use FileManager::getCanonicalName(), which expands all indirections
- // with llvm::sys::fs::real_path() and caches the result.
- //
- // On the other hand, it would be better to preserve as much of the
- // original path as possible, because that helps a user to recognize it.
- // real_path() expands all links, which is sometimes too much. Luckily,
- // on Windows we can just use llvm::sys::path::remove_dots(), because,
- // on that system, both aforementioned paths point to the same place.
-#ifdef _WIN32
- SmallString<256> TmpFilename = File->getName();
- llvm::sys::fs::make_absolute(TmpFilename);
- llvm::sys::path::native(TmpFilename);
- llvm::sys::path::remove_dots(TmpFilename, /* remove_dot_dot */ true);
- Filename = StringRef(TmpFilename.data(), TmpFilename.size());
-#else
- Filename = SM.getFileManager().getCanonicalName(*File);
-#endif
- }
- }
-
- return Filename;
-}
+// llvm::StringRef SARIFDiagnostic::EmitFilename(StringRef Filename,
+// const SourceManager &SM) {
+// if (DiagOpts.AbsolutePath) {
+// auto File = SM.getFileManager().getOptionalFileRef(Filename);
+// if (File) {
+// // We want to print a simplified absolute path, i. e. without "dots".
+// //
+// // The hardest part here are the paths like
"<part1>/<link>/../<part2>".
+// // On Unix-like systems, we cannot just collapse "<link>/..", because
+// // paths are resolved sequentially, and, thereby, the path
+// // "<part1>/<part2>" may point to a different location. That is why
+// // we use FileManager::getCanonicalName(), which expands all
indirections
+// // with llvm::sys::fs::real_path() and caches the result.
+// //
+// // On the other hand, it would be better to preserve as much of the
+// // original path as possible, because that helps a user to recognize
it.
+// // real_path() expands all links, which is sometimes too much.
Luckily,
+// // on Windows we can just use llvm::sys::path::remove_dots(), because,
+// // on that system, both aforementioned paths point to the same place.
+// #ifdef _WIN32
+// SmallString<256> TmpFilename = File->getName();
+// llvm::sys::fs::make_absolute(TmpFilename);
+// llvm::sys::path::native(TmpFilename);
+// llvm::sys::path::remove_dots(TmpFilename, /* remove_dot_dot */ true);
+// Filename = StringRef(TmpFilename.data(), TmpFilename.size());
+// #else
+// Filename = SM.getFileManager().getCanonicalName(*File);
+// #endif
+// }
+// }
+
+// return Filename;
+// }
/// Print out the file/line/column information and include trace.
///
>From 0156c90320ed07b672d98afdf0507a5df76ae883 Mon Sep 17 00:00:00 2001
From: anonymous <[email protected]>
Date: Thu, 1 Jan 2026 02:10:42 +0800
Subject: [PATCH 2/5] Physically define sarif-tree.
**Physically**, the sarif diagnostics format has a plain structure, that is:
- one `result` may carry several `Location`s and several `relatedLocation`s
- a `Location` is the place where error occurs, e.g. `main.cpp:3:4`,
- a `relatedLocation` is either a
- `In file included from...` location, or a
- *`child-result`*, who takes message, location, and **nesting**.
- which **logically** corresponds a `clang++` note diagnostic.
In this commit, we expand the definition of `SarifResult::RelatedLocations`
from `vector<CharSourceRange>` into `vector<variant<SarifChildResult,
CharSourceRange>>`, and, at the same time update the corresponding
serialization function for `relatedLocations`.
*(Note that GCC also makes sarif output in the same way: put child `results`
into `relatedLocations` and then mark them with `properties.nestingLevel`)*.
---
clang/include/clang/Basic/Sarif.h | 43 ++++++++++++++++++++++++++++++-
clang/lib/Basic/Sarif.cpp | 18 +++++++++++--
2 files changed, 58 insertions(+), 3 deletions(-)
diff --git a/clang/include/clang/Basic/Sarif.h
b/clang/include/clang/Basic/Sarif.h
index 6f82d253876d9..56080f69a3a1d 100644
--- a/clang/include/clang/Basic/Sarif.h
+++ b/clang/include/clang/Basic/Sarif.h
@@ -295,6 +295,42 @@ class SarifRule {
}
};
+/// A SARIF child-result is
+/// - logically, a sub-result (e.g. note) belonging to a SARIF result (e.g.
error, warning)
+/// - physically, a RelatedLocation in a SARIF result (same level as "in file
included from..." locations)
+class SarifChildResult {
+ friend class clang::SarifDocumentWriter;
+
+ std::string DiagnosticMessage;
+ llvm::SmallVector<CharSourceRange> Locations;
+ int Nesting;
+ std::optional<SarifResultLevel> LevelOverride;
+
+public:
+ static SarifChildResult create() { return SarifChildResult(); }
+
+ SarifChildResult setDiagnosticMessage(llvm::StringRef Message) {
+ DiagnosticMessage = Message.str();
+ return *this;
+ }
+
+ SarifChildResult addLocations(llvm::ArrayRef<CharSourceRange> DiagLocs) {
+#ifndef NDEBUG
+ for (const auto &Loc : DiagLocs) {
+ assert(Loc.isCharRange() &&
+ "SARIF Child Results require character granular source ranges!");
+ }
+#endif
+ Locations.append(DiagLocs.begin(), DiagLocs.end());
+ return *this;
+ }
+
+ SarifChildResult setNesting(int Nest) {
+ Nesting = Nest;
+ return *this;
+ }
+};
+
/// A SARIF result (also called a "reporting item") is a unit of output
/// produced when one of the tool's \c reportingDescriptor encounters a match
/// on the file being analysed by the tool.
@@ -325,7 +361,7 @@ class SarifResult {
std::string HostedViewerURI;
llvm::SmallDenseMap<StringRef, std::string, 4> PartialFingerprints;
llvm::SmallVector<CharSourceRange, 8> Locations;
- llvm::SmallVector<CharSourceRange, 8> RelatedLocations;
+ llvm::SmallVector<std::variant<SarifChildResult, CharSourceRange>, 8>
RelatedLocations; // A RelatedLocation is either a ChildResult or a plain "in
file included from..." Location.
llvm::SmallVector<ThreadFlow, 8> ThreadFlows;
std::optional<SarifResultLevel> LevelOverride;
@@ -378,6 +414,11 @@ class SarifResult {
return *this;
}
+ SarifResult addRelatedLocations(llvm::ArrayRef<SarifChildResult>
ChildResults) {
+ RelatedLocations.append(ChildResults.begin(), ChildResults.end());
+ return *this;
+ }
+
SarifResult setThreadFlows(llvm::ArrayRef<ThreadFlow> ThreadFlowResults) {
ThreadFlows.assign(ThreadFlowResults.begin(), ThreadFlowResults.end());
return *this;
diff --git a/clang/lib/Basic/Sarif.cpp b/clang/lib/Basic/Sarif.cpp
index 448de96d474af..1da7c6731d650 100644
--- a/clang/lib/Basic/Sarif.cpp
+++ b/clang/lib/Basic/Sarif.cpp
@@ -406,8 +406,22 @@ void SarifDocumentWriter::appendResult(const SarifResult
&Result) {
if (!Result.RelatedLocations.empty()) {
json::Array ReLocs;
- for (auto &Range : Result.RelatedLocations) {
- ReLocs.emplace_back(createLocation(createPhysicalLocation(Range)));
+ for (auto &RelatedLocation : Result.RelatedLocations) {
+ if (RelatedLocation.index() == 0) { // variant is a SarifChildResult
+ const SarifChildResult& ChildResult = std::get<0>(RelatedLocation);
+ json::Object Object;
+ Object.insert({"message",
createMessage(ChildResult.DiagnosticMessage)});
+ if (ChildResult.Locations.size() >= 1)
+ for (auto& kv :
createLocation(createPhysicalLocation(ChildResult.Locations[0])))
+ Object.insert({kv.getFirst(), kv.getSecond()});
+ Object.insert({
+ "properties", json::Object{{"nestingLevel", ChildResult.Nesting}}
+ });
+ ReLocs.emplace_back(std::move(Object));
+ } else { // variant is a CharSourceRange
+ const CharSourceRange& Range = std::get<1>(RelatedLocation);
+ ReLocs.emplace_back(createLocation(createPhysicalLocation(Range)));
+ }
}
Ret["relatedLocations"] = std::move(ReLocs);
}
>From 7e18dab89b04566291a60f02b9cf1ded8e771a55 Mon Sep 17 00:00:00 2001
From: anonymous <[email protected]>
Date: Thu, 1 Jan 2026 02:13:29 +0800
Subject: [PATCH 3/5] Adjust the printer logic.
Now the `SARIFDiagnotic` flushes all the diagnostics (at one time) when it
destructs, so we need to adjust the sequence of `SARIFDiagnosticPrinter` in
order that the `Writer->endRun()` happens **after** the `SARIFDiagnostic`
flushes.
---
clang/lib/Frontend/SARIFDiagnosticPrinter.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
b/clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
index 988159693389b..2d1225c823c86 100644
--- a/clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
+++ b/clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
@@ -38,11 +38,11 @@ void SARIFDiagnosticPrinter::BeginSourceFile(const
LangOptions &LO,
void SARIFDiagnosticPrinter::EndSourceFile() {
assert(SARIFDiag && "SARIFDiagnostic has not been set.");
- Writer->endRun();
+ SARIFDiag.reset();
llvm::json::Value Value(Writer->createDocument());
OS << "\n" << Value << "\n\n";
OS.flush();
- SARIFDiag.reset();
+ Writer->endRun();
}
void SARIFDiagnosticPrinter::HandleDiagnostic(DiagnosticsEngine::Level Level,
>From c2e9f6f0893eb78d042ff2955c54aa0c38037c6e Mon Sep 17 00:00:00 2001
From: anonymous <[email protected]>
Date: Thu, 1 Jan 2026 02:17:17 +0800
Subject: [PATCH 4/5] Disable sarif warnings.
Currently the `-Wsarif-format-unstable` warning happens **before** the
`SARIFDiagnostic` works, This warning message will not appear in the `json`
output, instead, it triggers in text-mode before the json output. To make the
`clang++` output machine-parsable, let's remove this warning (or move this
warning into the json).
In my perspective, after many efforts now the sarif mode seems ok to be used.
Let's remove the `-Wsarif-format-unstable` warning.
---
clang/include/clang/Basic/DiagnosticDriverKinds.td | 4 ----
clang/lib/Driver/ToolChains/Clang.cpp | 3 ---
2 files changed, 7 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 5cae8fb86347f..b0a31905c1930 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -851,10 +851,6 @@ def err_drv_invalid_empty_dxil_validator_version : Error<
"invalid validator version : %0; if validator major version is 0, minor "
"version must also be 0">;
-def warn_drv_sarif_format_unstable : Warning<
- "diagnostic formatting in SARIF mode is currently unstable">,
- InGroup<DiagGroup<"sarif-format-unstable">>;
-
def warn_drv_loongarch_conflicting_implied_val : Warning<
"ignoring '%0' as it conflicts with that implied by '%1' (%2)">,
InGroup<OptionIgnored>;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp
b/clang/lib/Driver/ToolChains/Clang.cpp
index 310f3b58a211e..38bcc12171143 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4230,9 +4230,6 @@ static void RenderDiagnosticsOptions(const Driver &D,
const ArgList &Args,
if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
CmdArgs.push_back("-fdiagnostics-format");
CmdArgs.push_back(A->getValue());
- if (StringRef(A->getValue()) == "sarif" ||
- StringRef(A->getValue()) == "SARIF")
- D.Diag(diag::warn_drv_sarif_format_unstable);
}
if (const Arg *A = Args.getLastArg(
>From be51f29c15fd1042e2a897337226399871ff3802 Mon Sep 17 00:00:00 2001
From: anonymous <[email protected]>
Date: Thu, 1 Jan 2026 02:18:27 +0800
Subject: [PATCH 5/5] Update test.
Update these corresponding test for `SARIFDiagnostic` and the
`relatedLocations` (with child results).
---
clang/unittests/Basic/SarifTest.cpp | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/clang/unittests/Basic/SarifTest.cpp
b/clang/unittests/Basic/SarifTest.cpp
index 42e85085d646e..74dd2d12b9446 100644
--- a/clang/unittests/Basic/SarifTest.cpp
+++ b/clang/unittests/Basic/SarifTest.cpp
@@ -290,7 +290,7 @@ TEST_F(SarifDocumentWriterTest,
checkSerializingResultsWithCustomRuleConfig) {
TEST_F(SarifDocumentWriterTest, checkSerializingArtifacts) {
// GIVEN:
const std::string ExpectedOutput =
-
R"({"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":40,"location":{"index":0,"uri":"file:///main.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0,"uri":"file:///main.cpp"},"region":{"endColumn":14,"startColumn":14,"startLine":3}}}],"message":{"text":"expected
';' after top level
declarator"},"relatedLocations":[{"physicalLocation":{"artifactLocation":{"index":0,"uri":"file:///main.cpp"},"region":{"endColumn":14,"startColumn":14,"startLine":3}}}],"ruleId":"clang.unittest","ruleIndex":0}],"tool":{"driver":{"fullName":"sarif
test
runner","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"sarif
test","rules":[{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":"Example
rule created during unit tests"},"id":"clang.unittest","name":"clang unit
test"}],"version":"1.0.0"}}}],"version":"2.1.0"})";
+
"{\"$schema\":\"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json\",\"runs\":[{\"artifacts\":[{\"length\":40,\"location\":{\"index\":0,\"uri\":\"file:///main.cpp\"},\"mimeType\":\"text/plain\",\"roles\":[\"resultFile\"]}],\"columnKind\":\"unicodeCodePoints\",\"results\":[{\"level\":\"error\",\"locations\":[{\"physicalLocation\":{\"artifactLocation\":{\"index\":0,\"uri\":\"file:///main.cpp\"},\"region\":{\"endColumn\":14,\"startColumn\":14,\"startLine\":3}}}],\"message\":{\"text\":\"expected
';' after top level
declarator\"},\"relatedLocations\":[{\"physicalLocation\":{\"artifactLocation\":{\"index\":0,\"uri\":\"file:///main.cpp\"},\"region\":{\"endColumn\":14,\"startColumn\":14,\"startLine\":3}}},{\"message\":{\"text\":\"in
instantiation of 'float x =
0.0'\"},\"physicalLocation\":{\"artifactLocation\":{\"index\":0,\"uri\":\"file:///main.cpp\"},\"region\":{\"endColumn\":14,\"startColumn\":14,\"startLine\":3}},\"properties\":{\"nestingLevel\":1}}],\"ruleId\":\"clang.unittest\",\"ruleIndex\":0}],\"tool\":{\"driver\":{\"fullName\":\"sarif
test
runner\",\"informationUri\":\"https://clang.llvm.org/docs/UsersManual.html\",\"language\":\"en-US\",\"name\":\"sarif
test\",\"rules\":[{\"defaultConfiguration\":{\"enabled\":true,\"level\":\"warning\",\"rank\":-1},\"fullDescription\":{\"text\":\"Example
rule created during unit tests\"},\"id\":\"clang.unittest\",\"name\":\"clang
unit test\"}],\"version\":\"1.0.0\"}}}],\"version\":\"2.1.0\"}";
SarifDocumentWriter Writer{SourceMgr};
const SarifRule &Rule =
@@ -316,10 +316,16 @@ TEST_F(SarifDocumentWriterTest,
checkSerializingArtifacts) {
const SarifResult &Result =
SarifResult::create(RuleIdx)
+ .setDiagnosticLevel(SarifResultLevel::Error)
+ .setDiagnosticMessage("expected ';' after top level declarator")
.addLocations(DiagLocs)
.addRelatedLocations(DiagLocs)
- .setDiagnosticMessage("expected ';' after top level declarator")
- .setDiagnosticLevel(SarifResultLevel::Error);
+ .addRelatedLocations({
+ SarifChildResult::create()
+ .setDiagnosticMessage("in instantiation of 'float x = 0.0'")
+ .addLocations(DiagLocs)
+ .setNesting(1)
+ });
Writer.appendResult(Result);
std::string Output = serializeSarifDocument(Writer.createDocument());
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
