https://github.com/anonymouspc updated https://github.com/llvm/llvm-project/pull/174106
>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/7] 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/7] 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/7] 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/7] 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/7] 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()); >From 575d22addab93e9c8c80b2382fd2314796dfe44e Mon Sep 17 00:00:00 2001 From: anonymous <[email protected]> Date: Thu, 1 Jan 2026 02:42:38 +0800 Subject: [PATCH 6/7] clang-format --- clang/include/clang/Basic/Sarif.h | 13 +- .../include/clang/Frontend/SARIFDiagnostic.h | 107 +++++++-------- clang/lib/Basic/Sarif.cpp | 15 ++- clang/lib/Frontend/SARIFDiagnostic.cpp | 124 +++++++++++------- clang/unittests/Basic/SarifTest.cpp | 41 +++++- 5 files changed, 178 insertions(+), 122 deletions(-) diff --git a/clang/include/clang/Basic/Sarif.h b/clang/include/clang/Basic/Sarif.h index 56080f69a3a1d..75921cae087da 100644 --- a/clang/include/clang/Basic/Sarif.h +++ b/clang/include/clang/Basic/Sarif.h @@ -296,8 +296,10 @@ 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) +/// - 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; @@ -361,7 +363,9 @@ class SarifResult { std::string HostedViewerURI; llvm::SmallDenseMap<StringRef, std::string, 4> PartialFingerprints; llvm::SmallVector<CharSourceRange, 8> Locations; - llvm::SmallVector<std::variant<SarifChildResult, CharSourceRange>, 8> RelatedLocations; // A RelatedLocation is either a ChildResult or a plain "in file included from..." Location. + 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; @@ -414,7 +418,8 @@ class SarifResult { return *this; } - SarifResult addRelatedLocations(llvm::ArrayRef<SarifChildResult> ChildResults) { + SarifResult + addRelatedLocations(llvm::ArrayRef<SarifChildResult> ChildResults) { RelatedLocations.append(ChildResults.begin(), ChildResults.end()); return *this; } diff --git a/clang/include/clang/Frontend/SARIFDiagnostic.h b/clang/include/clang/Frontend/SARIFDiagnostic.h index 70b5c80b68812..a396e645cb505 100644 --- a/clang/include/clang/Frontend/SARIFDiagnostic.h +++ b/clang/include/clang/Frontend/SARIFDiagnostic.h @@ -60,62 +60,63 @@ class SARIFDiagnostic : public DiagnosticRenderer { private: 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 = {}; + 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. + Node *Current = &Root; + SarifDocumentWriter + *Writer; // Shared between SARIFDiagnosticPrinter and this renderer. }; } // end namespace clang diff --git a/clang/lib/Basic/Sarif.cpp b/clang/lib/Basic/Sarif.cpp index 1da7c6731d650..bf7a603606a4c 100644 --- a/clang/lib/Basic/Sarif.cpp +++ b/clang/lib/Basic/Sarif.cpp @@ -408,18 +408,19 @@ void SarifDocumentWriter::appendResult(const SarifResult &Result) { json::Array ReLocs; for (auto &RelatedLocation : Result.RelatedLocations) { if (RelatedLocation.index() == 0) { // variant is a SarifChildResult - const SarifChildResult& ChildResult = std::get<0>(RelatedLocation); + const SarifChildResult &ChildResult = std::get<0>(RelatedLocation); json::Object Object; - Object.insert({"message", createMessage(ChildResult.DiagnosticMessage)}); + Object.insert( + {"message", createMessage(ChildResult.DiagnosticMessage)}); if (ChildResult.Locations.size() >= 1) - for (auto& kv : createLocation(createPhysicalLocation(ChildResult.Locations[0]))) + for (auto &kv : + createLocation(createPhysicalLocation(ChildResult.Locations[0]))) Object.insert({kv.getFirst(), kv.getSecond()}); - Object.insert({ - "properties", json::Object{{"nestingLevel", ChildResult.Nesting}} - }); + 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); + const CharSourceRange &Range = std::get<1>(RelatedLocation); ReLocs.emplace_back(createLocation(createPhysicalLocation(Range))); } } diff --git a/clang/lib/Frontend/SARIFDiagnostic.cpp b/clang/lib/Frontend/SARIFDiagnostic.cpp index 930d1b1383ddb..1e35d6df4e095 100644 --- a/clang/lib/Frontend/SARIFDiagnostic.cpp +++ b/clang/lib/Frontend/SARIFDiagnostic.cpp @@ -26,9 +26,9 @@ namespace clang { -// In sarif mode, -// a diagnostics 'group' have 1 top-level error/warning and several sub-level notes. -// For example: +// 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'. @@ -46,24 +46,24 @@ namespace clang { SARIFDiagnostic::SARIFDiagnostic(raw_ostream &OS, const LangOptions &LangOpts, DiagnosticOptions &DiagOpts, SarifDocumentWriter *Writer) - : DiagnosticRenderer(LangOpts, DiagOpts), - Root(Node::Result(), Node::Option{&LangOpts, &DiagOpts}, /*Nesting=*/-1), // The root does not represents a diagnostic. - Current(&Root), - 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); - } +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() { @@ -115,62 +115,78 @@ void SARIFDiagnostic::emitDiagnosticMessage( DiagOrStoredDiag Diag) { if (Level >= DiagnosticsEngine::Level::Warning) { - Current = &Root; // If this is a top-level error/warning, repoint Current to Root. + 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 + ->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)}); + 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)}); } void SARIFDiagnostic::emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc) { Current = &Current->addRelatedLocation(Node::Location{Loc, PLoc, {}}); } -void SARIFDiagnostic::emitImportLocation(FullSourceLoc Loc, PresumedLoc PLoc, StringRef ModuleName) { +void SARIFDiagnostic::emitImportLocation(FullSourceLoc Loc, PresumedLoc PLoc, + StringRef ModuleName) { Current = &Current->addRelatedLocation(Node::Location{Loc, PLoc, {}}); } SARIFDiagnostic::Node::Node(Result Result_, Option Option_, int Nesting) - : Result_(std::move(Result_)), Option_(std::move(Option_)), Nesting(Nesting) {} + : Result_(std::move(Result_)), Option_(std::move(Option_)), + Nesting(Nesting) {} -SARIFDiagnostic::Node& SARIFDiagnostic::Node::getParent() { +SARIFDiagnostic::Node &SARIFDiagnostic::Node::getParent() { assert(ParentPtr && "getParent() of SARIFDiagnostic::Root!"); return *ParentPtr; } -SARIFDiagnostic::Node& SARIFDiagnostic::Node::getForkableParent() { - Node* Ptr = this; - while (Ptr->getLevel() <= DiagnosticsEngine::Note) // The forkable node here "is and only is" warning/error/fatal. +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; } -llvm::SmallVector<std::unique_ptr<SARIFDiagnostic::Node>>& SARIFDiagnostic::Node::getChildrenPtrs() { +llvm::SmallVector<std::unique_ptr<SARIFDiagnostic::Node>> & +SARIFDiagnostic::Node::getChildrenPtrs() { return ChildrenPtrs; } -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)); +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(); } -SARIFDiagnostic::Node& SARIFDiagnostic::Node::addLocation(Location Location) { +SARIFDiagnostic::Node &SARIFDiagnostic::Node::addLocation(Location Location) { Locations.push_back(std::move(Location)); return *this; } -SARIFDiagnostic::Node& SARIFDiagnostic::Node::addRelatedLocation(Location Location) { +SARIFDiagnostic::Node & +SARIFDiagnostic::Node::addRelatedLocation(Location Location) { RelatedLocations.push_back(std::move(Location)); return *this; } 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(); + return llvm::isa<const Diagnostic *>(Result_.Diag) + ? Result_.Diag.dyn_cast<const Diagnostic *>()->getID() + : Result_.Diag.dyn_cast<const StoredDiagnostic *>()->getID(); } DiagnosticsEngine::Level SARIFDiagnostic::Node::getLevel() { @@ -183,34 +199,36 @@ std::string SARIFDiagnostic::Node::getDiagnosticMessage() { llvm::SmallVector<CharSourceRange> SARIFDiagnostic::Node::getLocations() { llvm::SmallVector<CharSourceRange> CharSourceRanges; - std::for_each(Locations.begin(), Locations.end(), [&] (Location& Location) { - CharSourceRanges.append(Location.getCharSourceRangesWithOption(Option_)); + 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> +SARIFDiagnostic::Node::getRelatedLocations() { llvm::SmallVector<CharSourceRange> CharSourceRanges; - std::for_each(RelatedLocations.begin(), RelatedLocations.end(), [&] (Location& RelatedLocation) { - CharSourceRanges.append(RelatedLocation.getCharSourceRangesWithOption(Option_)); - }); + std::for_each(RelatedLocations.begin(), RelatedLocations.end(), + [&](Location &RelatedLocation) { + CharSourceRanges.append( + RelatedLocation.getCharSourceRangesWithOption(Option_)); + }); return CharSourceRanges; } -int SARIFDiagnostic::Node::getNesting() { - return Nesting; -} +int SARIFDiagnostic::Node::getNesting() { return Nesting; } -llvm::SmallVector<CharSourceRange> SARIFDiagnostic::Node::Location::getCharSourceRangesWithOption(Option Option) { +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()); + // EmitFilename(FE->getName(), Loc.getManager()); } } return {}; @@ -258,10 +276,11 @@ llvm::SmallVector<CharSourceRange> SARIFDiagnostic::Node::Location::getCharSourc auto &SM = Loc.getManager(); auto FID = PLoc.getFileID(); // Visual Studio 2010 or earlier expects column number to be off by one. - unsigned int ColNo = (Option.LangOptsPtr->MSCompatibilityVersion && - !Option.LangOptsPtr->isCompatibleWithMSVC(LangOptions::MSVC2012)) - ? PLoc.getColumn() - 1 - : PLoc.getColumn(); + unsigned int ColNo = + (Option.LangOptsPtr->MSCompatibilityVersion && + !Option.LangOptsPtr->isCompatibleWithMSVC(LangOptions::MSVC2012)) + ? PLoc.getColumn() - 1 + : PLoc.getColumn(); SourceLocation DiagLoc = SM.translateLineCol(FID, PLoc.getLine(), ColNo); // FIXME(llvm-project/issues/57366): Properly process #line directives. @@ -279,15 +298,18 @@ llvm::SmallVector<CharSourceRange> SARIFDiagnostic::Node::Location::getCharSourc // 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>". +// // 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 +// // 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. +// // 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. diff --git a/clang/unittests/Basic/SarifTest.cpp b/clang/unittests/Basic/SarifTest.cpp index 74dd2d12b9446..bc19a74ba9148 100644 --- a/clang/unittests/Basic/SarifTest.cpp +++ b/clang/unittests/Basic/SarifTest.cpp @@ -290,7 +290,35 @@ TEST_F(SarifDocumentWriterTest, checkSerializingResultsWithCustomRuleConfig) { TEST_F(SarifDocumentWriterTest, checkSerializingArtifacts) { // GIVEN: const std::string ExpectedOutput = - "{\"$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\"}"; + "{\"$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 = @@ -320,12 +348,11 @@ TEST_F(SarifDocumentWriterTest, checkSerializingArtifacts) { .setDiagnosticMessage("expected ';' after top level declarator") .addLocations(DiagLocs) .addRelatedLocations(DiagLocs) - .addRelatedLocations({ - SarifChildResult::create() - .setDiagnosticMessage("in instantiation of 'float x = 0.0'") - .addLocations(DiagLocs) - .setNesting(1) - }); + .addRelatedLocations( + {SarifChildResult::create() + .setDiagnosticMessage("in instantiation of 'float x = 0.0'") + .addLocations(DiagLocs) + .setNesting(1)}); Writer.appendResult(Result); std::string Output = serializeSarifDocument(Writer.createDocument()); >From 9293d32a740c3424130f5bc7f2e4b46da37dd499 Mon Sep 17 00:00:00 2001 From: anonymous <[email protected]> Date: Thu, 1 Jan 2026 02:48:36 +0800 Subject: [PATCH 7/7] Update release notes --- clang/docs/ReleaseNotes.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 2319ff13f7864..411e2acd8933a 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -495,6 +495,10 @@ Improvements to Clang's diagnostics carries messages like 'In file included from ...' or 'In module ...'. Now the include/import locations are written into `sarif.run.result.relatedLocations`. +- Add nested/indented diagnose support when enabling ``-fdiagnostics-format=sarif``. + Now the notes are recursively mounted under error/warnings, and the diagnostics can + be parsed into a nested/indented tree. + - Clang now generates a fix-it for C++20 designated initializers when the initializers do not match the declaration order in the structure. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
