https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/214461
From 20ef11ea67f839b5a9c176c6742ac3d5f2feaafd Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Thu, 6 Aug 2026 11:02:08 +0100 Subject: [PATCH] [clang] Reject ranges getExpansionRangeInFile cannot represent getExpansionRangeInFile was extracted verbatim and inherited two shortcomings of the original loop, fixed here before the analyzer's SARIF and HTML consumers depend on it: - It mapped the end with getExpansionRange(SourceLocation), which always reports a token range, so a char-range input was widened by a whole token. Now using the getExpansionRange(CharSourceRange) overload, which keeps the flag. - It passed reversed ranges through. Consumers walk begin->end; now returning nullopt for those, as Lexer::makeFileCharRange already does. Separate from the extraction so that stays NFC, and out of the consumer fixes because it changes the shared helper's contract rather than one output. Both contract changes, plus the invalid- and cross-file-range guards, are covered by a GetExpansionRangeInFile unit test in clang/unittests/Frontend/TextDiagnosticTest.cpp. Assisted-By: claude --- .../clang/Frontend/DiagnosticRenderer.h | 9 +- clang/lib/Frontend/DiagnosticRenderer.cpp | 6 ++ .../unittests/Frontend/TextDiagnosticTest.cpp | 89 +++++++++++++++++++ 3 files changed, 103 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Frontend/DiagnosticRenderer.h b/clang/include/clang/Frontend/DiagnosticRenderer.h index 0b123a87131ff..43f99c383d94a 100644 --- a/clang/include/clang/Frontend/DiagnosticRenderer.h +++ b/clang/include/clang/Frontend/DiagnosticRenderer.h @@ -36,7 +36,14 @@ 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. +/// \returns nullopt if \p Range is invalid, if an endpoint lies outside \p FID, +/// Unlike \c Lexer::makeFileCharRange(), which gives up when an endpoint is +/// strictly inside an expansion, this points at the expansion; prefer +/// \c makeFileCharRange() when a faithful file range matters, e.g. for +/// rewriting. +/// +/// The result may still be a token range, but \c Lexer::getAsCharRange() cannot +/// fail on it: both endpoints are file locations. std::optional<CharSourceRange> getExpansionRangeInFile(CharSourceRange Range, FileID FID, const SourceManager &SM); diff --git a/clang/lib/Frontend/DiagnosticRenderer.cpp b/clang/lib/Frontend/DiagnosticRenderer.cpp index 910aa1054b602..67760e2b7c4dd 100644 --- a/clang/lib/Frontend/DiagnosticRenderer.cpp +++ b/clang/lib/Frontend/DiagnosticRenderer.cpp @@ -46,6 +46,12 @@ clang::getExpansionRangeInFile(CharSourceRange Range, FileID FID, return std::nullopt; } + // Both endpoints are in FID, so comparing their offsets is meaningful. + if (SM.getFileOffset(Expansion.getBegin()) > + SM.getFileOffset(Expansion.getEnd())) { + return std::nullopt; + } + return Expansion; } diff --git a/clang/unittests/Frontend/TextDiagnosticTest.cpp b/clang/unittests/Frontend/TextDiagnosticTest.cpp index 4c4decc4a6857..6aec5224d00a7 100644 --- a/clang/unittests/Frontend/TextDiagnosticTest.cpp +++ b/clang/unittests/Frontend/TextDiagnosticTest.cpp @@ -9,10 +9,13 @@ #include "clang/Frontend/TextDiagnostic.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/LangOptions.h" +#include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" +#include "clang/Frontend/DiagnosticRenderer.h" #include "clang/Frontend/TextDiagnosticPrinter.h" #include "llvm/Support/SmallVectorMemoryBuffer.h" #include "gtest/gtest.h" +#include <optional> using namespace llvm; using namespace clang; @@ -121,4 +124,90 @@ TEST_P(ShowLevelNoLocationTest, LevelPrefixRespected) { INSTANTIATE_TEST_SUITE_P(ShowLevelNoLocation, ShowLevelNoLocationTest, ::testing::Bool()); +// Creates a virtual file with the given contents and returns its FileID. +static FileID makeFile(FileManager &FileMgr, SourceManager &SrcMgr, + StringRef Path, StringRef Contents) { + FileEntryRef FE = FileMgr.getVirtualFileRef( + Path, /*Size=*/static_cast<off_t>(Contents.size()), + /*ModificationTime=*/0); + SmallVector<char, 64> Buffer(Contents.begin(), Contents.end()); + SrcMgr.overrideFileContents(FE, std::make_unique<SmallVectorMemoryBuffer>( + std::move(Buffer), Path, + /*RequiresNullTerminator=*/false)); + return SrcMgr.createFileID(FE, SourceLocation(), SrcMgr::C_User); +} + +TEST(DiagnosticRenderer, GetExpansionRangeInFileTest) { + FileSystemOptions FSOpts; + FileManager FileMgr(FSOpts); + DiagnosticOptions DiagEngineOpts; + DiagnosticsEngine DiagEngine(DiagnosticIDs::create(), DiagEngineOpts, + new IgnoringDiagConsumer()); + SourceManager SM(DiagEngine, FileMgr); + + FileID FID = makeFile(FileMgr, SM, "main.cpp", "some\nsource\ncode\n"); + FileID OtherFID = makeFile(FileMgr, SM, "other.cpp", "other\n"); + SM.setMainFileID(FID); + + auto Loc = [&](unsigned Line, unsigned Col) { + return SM.translateLineCol(FID, Line, Col); + }; + + const SourceLocation L1C1 = Loc(/*Line=*/1, /*Col=*/1); + const SourceLocation L1C3 = Loc(/*Line=*/1, /*Col=*/3); + + // An invalid range is rejected. + EXPECT_FALSE(getExpansionRangeInFile(CharSourceRange(), FID, SM)); + + // A char range stays a char range. + std::optional<CharSourceRange> CharR = getExpansionRangeInFile( + CharSourceRange::getCharRange(L1C1, L1C3), FID, SM); + ASSERT_TRUE(CharR); + EXPECT_TRUE(CharR->isCharRange()); + + // A token range stays a token range. + std::optional<CharSourceRange> TokR = getExpansionRangeInFile( + CharSourceRange::getTokenRange(L1C1, L1C3), FID, SM); + ASSERT_TRUE(TokR); + EXPECT_TRUE(TokR->isTokenRange()); + + // A reversed range (begin lies after end) is rejected. + EXPECT_FALSE(getExpansionRangeInFile( + CharSourceRange::getCharRange(L1C3, L1C1), FID, SM)); + + // A range with an endpoint in another file is rejected. + SourceLocation OtherLoc = SM.getLocForStartOfFile(OtherFID); + EXPECT_FALSE(getExpansionRangeInFile( + CharSourceRange::getTokenRange(L1C1, OtherLoc), FID, SM)); + + { + const SourceLocation L2C1 = Loc(/*Line=*/2, /*Col=*/1); + const SourceLocation L2C6 = Loc(/*Line=*/2, /*Col=*/6); + + // Pretend that "source" expands "some". + SourceLocation MacroLoc = SM.createExpansionLoc( + /*SpellingLoc=*/L1C1, /*ExpansionLocStart=*/L2C1, + /*ExpansionLocEnd=*/L2C6, /*Length=*/4); + ASSERT_TRUE(MacroLoc.isMacroID()); + ASSERT_EQ(SM.getSpellingLoc(MacroLoc), L1C1); + ASSERT_EQ(SM.getExpansionLoc(MacroLoc), L2C1); + + // A macro-expanded range is remapped to its expansion in the file. + // A location inside the macro maps back to that file range. + auto MacroToken = CharSourceRange::getTokenRange(MacroLoc, MacroLoc); + auto MacroR = getExpansionRangeInFile(MacroToken, FID, SM); + ASSERT_TRUE(MacroR); + EXPECT_EQ(SM.getFileID(MacroR->getBegin()), FID); + EXPECT_EQ(SM.getFileID(MacroR->getEnd()), FID); + + // The range is a file range. + EXPECT_TRUE(MacroR->getBegin().isFileID()); + EXPECT_TRUE(MacroR->getEnd().isFileID()); + + // The range is the expansion range. + EXPECT_EQ(MacroR->getBegin(), L2C1); + EXPECT_EQ(MacroR->getEnd(), L2C6); + } +} + } // anonymous namespace _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
