https://github.com/steakhal created https://github.com/llvm/llvm-project/pull/214460
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 --- <sub>Stack created with <a href="https://github.com/github/gh-stack";>GitHub Stacks CLI</a> • <a href="https://gh.io/stacks-feedback";>Give Feedback 💬</a></sub> From 9a086c9e2e6a01b0d86135290fa9a1a67c6e4a28 Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Thu, 6 Aug 2026 10:55:52 +0100 Subject: [PATCH] [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 | 15 ++++++++++++ clang/lib/Frontend/DiagnosticRenderer.cpp | 16 +++++++++++++ clang/lib/Frontend/SARIFDiagnostic.cpp | 24 ++++++------------- clang/lib/Frontend/TextDiagnostic.cpp | 17 +++++-------- 4 files changed, 44 insertions(+), 28 deletions(-) diff --git a/clang/include/clang/Frontend/DiagnosticRenderer.h b/clang/include/clang/Frontend/DiagnosticRenderer.h index 3f03a6e02da4b..6f9fe0422088f 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,20 @@ 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. Returns std::nullopt if \p Range is invalid or an +/// endpoint lies outside \p FID. +/// +/// Unlike Lexer::makeFileCharRange(), which gives up when an endpoint is +/// strictly inside an expansion, this points at the expansion; prefer +/// makeFileCharRange() when a faithful file range matters, e.g. for rewriting. +/// +/// The result may still be a token range, but Lexer::getAsCharRange() cannot +/// fail on it: both endpoints are file locations. +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); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
