llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Balázs Benics (steakhal) <details> <summary>Changes</summary> 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> --- Full diff: https://github.com/llvm/llvm-project/pull/214460.diff 4 Files Affected: - (modified) clang/include/clang/Frontend/DiagnosticRenderer.h (+7) - (modified) clang/lib/Frontend/DiagnosticRenderer.cpp (+16) - (modified) clang/lib/Frontend/SARIFDiagnostic.cpp (+7-17) - (modified) clang/lib/Frontend/TextDiagnostic.cpp (+6-11) ``````````diff 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); `````````` </details> https://github.com/llvm/llvm-project/pull/214460 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
