[llvm-branch-commits] [clang] [clang] Reject ranges getExpansionRangeInFile cannot represent (PR #214461)
Balázs Benics via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Aug 10 04:09:37 PDT 2026
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/214461
>From ee8dc4c03c3265b48c7ff5f59faff2330bc7e8de Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
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 | 11 ++-
clang/lib/Frontend/DiagnosticRenderer.cpp | 9 +-
.../unittests/Frontend/TextDiagnosticTest.cpp | 89 +++++++++++++++++++
3 files changed, 106 insertions(+), 3 deletions(-)
diff --git a/clang/include/clang/Frontend/DiagnosticRenderer.h b/clang/include/clang/Frontend/DiagnosticRenderer.h
index 7a4921e0a794d..622c068b8e61f 100644
--- a/clang/include/clang/Frontend/DiagnosticRenderer.h
+++ b/clang/include/clang/Frontend/DiagnosticRenderer.h
@@ -35,9 +35,16 @@ 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,
+/// or if the beginning of the range is after the end.
///
-/// \returns std::nullopt if \p Range is invalid, or if either endpoint maps
-/// outside \p FID - a range the caller cannot render against that file.
+/// 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 f7c4114cb5aa9..67760e2b7c4dd 100644
--- a/clang/lib/Frontend/DiagnosticRenderer.cpp
+++ b/clang/lib/Frontend/DiagnosticRenderer.cpp
@@ -42,8 +42,15 @@ 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;
+ }
+
+ // 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
More information about the llvm-branch-commits
mailing list