https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/214460
From a9207acd87fea7a5933312f33ac6a9e821200c58 Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Thu, 6 Aug 2026 10:55:52 +0100 Subject: [PATCH 1/4] [clang][NFC] Extract getExpansionRangeInFile out of the diagnostic renderers Prep for the following commits, which fix crashes in the analyzer's SARIF and HTML output on ranges that end inside a macro expansion. Fixing them means mapping such a range into the reported file - the normalization the frontend text and SARIF renderers already do, and that the two analyzer consumers each do differently and incorrectly. Hoist that logic into getExpansionRangeInFile, beside the DiagnosticRenderer base both frontend renderers derive from, so the fixes reuse one implementation instead of adding two more copies. TextDiagnostic and SARIFDiagnostic move onto it here with no behavior change; the analyzer consumers follow in later commits. getFileID() replaces SARIFDiagnostic's getDecomposedLoc(...).first - equivalent here, and what TextDiagnostic has used since c113cbb51005. Assisted-By: claude --- .../clang/Frontend/DiagnosticRenderer.h | 7 ++++++ clang/lib/Frontend/DiagnosticRenderer.cpp | 16 +++++++++++++ clang/lib/Frontend/SARIFDiagnostic.cpp | 24 ++++++------------- clang/lib/Frontend/TextDiagnostic.cpp | 17 +++++-------- 4 files changed, 36 insertions(+), 28 deletions(-) diff --git a/clang/include/clang/Frontend/DiagnosticRenderer.h b/clang/include/clang/Frontend/DiagnosticRenderer.h index 3f03a6e02da4b..06aa8aa35ebca 100644 --- a/clang/include/clang/Frontend/DiagnosticRenderer.h +++ b/clang/include/clang/Frontend/DiagnosticRenderer.h @@ -23,6 +23,7 @@ #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/PointerUnion.h" #include "llvm/ADT/StringRef.h" +#include <optional> namespace clang { @@ -32,6 +33,12 @@ class SourceManager; using DiagOrStoredDiag = llvm::PointerUnion<const Diagnostic *, const StoredDiagnostic *>; +/// Maps both endpoints of \p Range to their macro expansion, so that the range +/// can be shown to a user. +std::optional<CharSourceRange> getExpansionRangeInFile(CharSourceRange Range, + FileID FID, + const SourceManager &SM); + /// Class to encapsulate the logic for formatting a diagnostic message. /// /// Actual "printing" logic is implemented by subclasses. diff --git a/clang/lib/Frontend/DiagnosticRenderer.cpp b/clang/lib/Frontend/DiagnosticRenderer.cpp index 2b6fbc274e587..c0c6e15ec0378 100644 --- a/clang/lib/Frontend/DiagnosticRenderer.cpp +++ b/clang/lib/Frontend/DiagnosticRenderer.cpp @@ -34,6 +34,22 @@ DiagnosticRenderer::DiagnosticRenderer(const LangOptions &LangOpts, DiagnosticRenderer::~DiagnosticRenderer() = default; +std::optional<CharSourceRange> +clang::getExpansionRangeInFile(CharSourceRange Range, FileID FID, + const SourceManager &SM) { + if (Range.isInvalid()) + return std::nullopt; + + SourceLocation Begin = SM.getExpansionLoc(Range.getBegin()); + CharSourceRange EndRange = SM.getExpansionRange(Range.getEnd()); + SourceLocation End = EndRange.getEnd(); + + if (SM.getFileID(Begin) != FID || SM.getFileID(End) != FID) + return std::nullopt; + + return CharSourceRange(SourceRange(Begin, End), EndRange.isTokenRange()); +} + namespace { class FixitReceiver : public edit::EditsReceiver { diff --git a/clang/lib/Frontend/SARIFDiagnostic.cpp b/clang/lib/Frontend/SARIFDiagnostic.cpp index 04ee14edfe74c..5183462b76f41 100644 --- a/clang/lib/Frontend/SARIFDiagnostic.cpp +++ b/clang/lib/Frontend/SARIFDiagnostic.cpp @@ -120,29 +120,20 @@ SARIFDiagnostic::getSarifLocation(FullSourceLoc Loc, PresumedLoc PLoc, FileID CaretFileID = Loc.getExpansionLoc().getFileID(); + auto &SM = Loc.getManager(); for (const CharSourceRange Range : Ranges) { - // Ignore invalid ranges. - if (Range.isInvalid()) + std::optional<CharSourceRange> FileRange = + getExpansionRangeInFile(Range, CaretFileID, SM); + if (!FileRange) continue; - auto &SM = Loc.getManager(); - SourceLocation B = SM.getExpansionLoc(Range.getBegin()); - CharSourceRange ERange = SM.getExpansionRange(Range.getEnd()); - SourceLocation E = ERange.getEnd(); - bool IsTokenRange = ERange.isTokenRange(); - - FileIDAndOffset BInfo = SM.getDecomposedLoc(B); - FileIDAndOffset EInfo = SM.getDecomposedLoc(E); - - // If the start or end of the range is in another file, just discard - // it. - if (BInfo.first != CaretFileID || EInfo.first != CaretFileID) - continue; + SourceLocation B = FileRange->getBegin(); + SourceLocation E = FileRange->getEnd(); // Add in the length of the token, so that we cover multi-char // tokens. unsigned TokSize = 0; - if (IsTokenRange) + if (FileRange->isTokenRange()) TokSize = Lexer::MeasureTokenLength(E, SM, LangOpts); FullSourceLoc BF(B, SM), EF(E, SM); @@ -157,7 +148,6 @@ SARIFDiagnostic::getSarifLocation(FullSourceLoc Loc, PresumedLoc PLoc, // 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 && diff --git a/clang/lib/Frontend/TextDiagnostic.cpp b/clang/lib/Frontend/TextDiagnostic.cpp index 01a4d2f6392d3..5221ed4de02ff 100644 --- a/clang/lib/Frontend/TextDiagnostic.cpp +++ b/clang/lib/Frontend/TextDiagnostic.cpp @@ -921,23 +921,18 @@ void TextDiagnostic::emitDiagnosticLoc(FullSourceLoc Loc, PresumedLoc PLoc, const SourceManager &SM = Loc.getManager(); for (const auto &R : Ranges) { - // Ignore invalid ranges. - if (!R.isValid()) + std::optional<CharSourceRange> FileRange = + getExpansionRangeInFile(R, CaretFileID, SM); + if (!FileRange) continue; - SourceLocation B = SM.getExpansionLoc(R.getBegin()); - CharSourceRange ERange = SM.getExpansionRange(R.getEnd()); - SourceLocation E = ERange.getEnd(); - - // If the start or end of the range is in another file, just - // discard it. - if (SM.getFileID(B) != CaretFileID || SM.getFileID(E) != CaretFileID) - continue; + SourceLocation B = FileRange->getBegin(); + SourceLocation E = FileRange->getEnd(); // Add in the length of the token, so that we cover multi-char // tokens. unsigned TokSize = 0; - if (ERange.isTokenRange()) + if (FileRange->isTokenRange()) TokSize = Lexer::MeasureTokenLength(E, SM, LangOpts); FullSourceLoc BF(B, SM), EF(E, SM); From a117608cd1487401064079bd097622b22942e0d4 Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Mon, 10 Aug 2026 10:56:22 +0100 Subject: [PATCH 2/4] [clang][NFC] Reuse SourceManager::getExpansionRange in getExpansionRangeInFile --- clang/lib/Frontend/DiagnosticRenderer.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/clang/lib/Frontend/DiagnosticRenderer.cpp b/clang/lib/Frontend/DiagnosticRenderer.cpp index c0c6e15ec0378..f7c4114cb5aa9 100644 --- a/clang/lib/Frontend/DiagnosticRenderer.cpp +++ b/clang/lib/Frontend/DiagnosticRenderer.cpp @@ -40,14 +40,12 @@ clang::getExpansionRangeInFile(CharSourceRange Range, FileID FID, if (Range.isInvalid()) return std::nullopt; - SourceLocation Begin = SM.getExpansionLoc(Range.getBegin()); - CharSourceRange EndRange = SM.getExpansionRange(Range.getEnd()); - SourceLocation End = EndRange.getEnd(); - - if (SM.getFileID(Begin) != FID || SM.getFileID(End) != FID) + CharSourceRange Expansion = SM.getExpansionRange(Range); + if (SM.getFileID(Expansion.getBegin()) != FID || + SM.getFileID(Expansion.getEnd()) != FID) return std::nullopt; - return CharSourceRange(SourceRange(Begin, End), EndRange.isTokenRange()); + return Expansion; } namespace { From f9e6d5ff00622b1970345028795920a2f71520d5 Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Mon, 10 Aug 2026 10:56:45 +0100 Subject: [PATCH 3/4] [clang][NFC] Document getExpansionRangeInFile's nullopt cases --- clang/include/clang/Frontend/DiagnosticRenderer.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clang/include/clang/Frontend/DiagnosticRenderer.h b/clang/include/clang/Frontend/DiagnosticRenderer.h index 06aa8aa35ebca..0b123a87131ff 100644 --- a/clang/include/clang/Frontend/DiagnosticRenderer.h +++ b/clang/include/clang/Frontend/DiagnosticRenderer.h @@ -35,6 +35,8 @@ using DiagOrStoredDiag = /// Maps both endpoints of \p Range to their macro expansion, so that the range /// can be shown to a user. +/// +/// \returns nullopt if \p Range is invalid, if an endpoint lies outside \p FID. std::optional<CharSourceRange> getExpansionRangeInFile(CharSourceRange Range, FileID FID, const SourceManager &SM); From 87b26f05a23c91875d01d411ab95c6214b3754e8 Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Mon, 10 Aug 2026 12:15:23 +0100 Subject: [PATCH 4/4] NFC Add braces --- clang/lib/Frontend/DiagnosticRenderer.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Frontend/DiagnosticRenderer.cpp b/clang/lib/Frontend/DiagnosticRenderer.cpp index f7c4114cb5aa9..910aa1054b602 100644 --- a/clang/lib/Frontend/DiagnosticRenderer.cpp +++ b/clang/lib/Frontend/DiagnosticRenderer.cpp @@ -42,8 +42,9 @@ clang::getExpansionRangeInFile(CharSourceRange Range, FileID FID, CharSourceRange Expansion = SM.getExpansionRange(Range); if (SM.getFileID(Expansion.getBegin()) != FID || - SM.getFileID(Expansion.getEnd()) != FID) + SM.getFileID(Expansion.getEnd()) != FID) { return std::nullopt; + } return Expansion; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
