[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
Thu Aug 6 06:17:57 PDT 2026
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/214461
>From 87db2cf1cd5961d7edb43593740b8669088aad19 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 | 4 +-
clang/lib/Frontend/DiagnosticRenderer.cpp | 13 ++--
.../unittests/Frontend/TextDiagnosticTest.cpp | 76 +++++++++++++++++++
3 files changed, 86 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/Frontend/DiagnosticRenderer.h b/clang/include/clang/Frontend/DiagnosticRenderer.h
index 6f9fe0422088f..2adec4da12cb2 100644
--- a/clang/include/clang/Frontend/DiagnosticRenderer.h
+++ b/clang/include/clang/Frontend/DiagnosticRenderer.h
@@ -34,8 +34,8 @@ 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.
+/// can be shown to a user. Returns std::nullopt if \p Range is invalid, if an
+/// endpoint lies outside \p FID, or if the endpoints come out reversed.
///
/// Unlike Lexer::makeFileCharRange(), which gives up when an endpoint is
/// strictly inside an expansion, this points at the expansion; prefer
diff --git a/clang/lib/Frontend/DiagnosticRenderer.cpp b/clang/lib/Frontend/DiagnosticRenderer.cpp
index c0c6e15ec0378..c8e9314d989f6 100644
--- a/clang/lib/Frontend/DiagnosticRenderer.cpp
+++ b/clang/lib/Frontend/DiagnosticRenderer.cpp
@@ -40,14 +40,17 @@ 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();
+ CharSourceRange Expansion = SM.getExpansionRange(Range);
+ if (SM.getFileID(Expansion.getBegin()) != FID ||
+ SM.getFileID(Expansion.getEnd()) != FID)
+ return std::nullopt;
- if (SM.getFileID(Begin) != FID || SM.getFileID(End) != FID)
+ // Both endpoints are in FID, so comparing their offsets is meaningful.
+ if (SM.getFileOffset(Expansion.getBegin()) >
+ SM.getFileOffset(Expansion.getEnd()))
return std::nullopt;
- return CharSourceRange(SourceRange(Begin, End), EndRange.isTokenRange());
+ return Expansion;
}
namespace {
diff --git a/clang/unittests/Frontend/TextDiagnosticTest.cpp b/clang/unittests/Frontend/TextDiagnosticTest.cpp
index 4c4decc4a6857..441e63f02ef2c 100644
--- a/clang/unittests/Frontend/TextDiagnosticTest.cpp
+++ b/clang/unittests/Frontend/TextDiagnosticTest.cpp
@@ -10,9 +10,11 @@
#include "clang/Basic/FileManager.h"
#include "clang/Basic/LangOptions.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 +123,78 @@ 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(GetExpansionRangeInFile, HandlesRangesTheContractRejectsOrRemaps) {
+ 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);
+ };
+
+ // An invalid range is rejected.
+ EXPECT_FALSE(getExpansionRangeInFile(CharSourceRange(), FID, SM));
+
+ // A char range whose end is a plain file location keeps its char flag. The
+ // old per-endpoint mapping went through getExpansionRange(SourceLocation),
+ // which always reports a token range, so this used to be widened by a whole
+ // token.
+ std::optional<CharSourceRange> CharR = getExpansionRangeInFile(
+ CharSourceRange::getCharRange(Loc(1, 1), Loc(1, 3)), FID, SM);
+ ASSERT_TRUE(CharR);
+ EXPECT_TRUE(CharR->isCharRange());
+
+ // A token range stays a token range.
+ std::optional<CharSourceRange> TokR = getExpansionRangeInFile(
+ CharSourceRange::getTokenRange(Loc(1, 1), Loc(1, 3)), FID, SM);
+ ASSERT_TRUE(TokR);
+ EXPECT_TRUE(TokR->isTokenRange());
+
+ // A reversed range (begin lies after end) is rejected rather than passed
+ // through for consumers to walk begin->end.
+ EXPECT_FALSE(getExpansionRangeInFile(
+ CharSourceRange::getCharRange(Loc(1, 3), Loc(1, 1)), FID, SM));
+
+ // A range with an endpoint in another file is rejected.
+ SourceLocation OtherLoc = SM.getLocForStartOfFile(OtherFID);
+ EXPECT_FALSE(getExpansionRangeInFile(
+ CharSourceRange::getTokenRange(Loc(1, 1), OtherLoc), FID, SM));
+
+ // A macro-expanded range is remapped to its expansion in the file. The macro
+ // use spans [Loc(2,1), Loc(2,6)] in main.cpp; a location inside the macro
+ // maps back to that file range.
+ SourceLocation MacroLoc = SM.createExpansionLoc(
+ /*SpellingLoc=*/Loc(1, 1), /*ExpansionLocStart=*/Loc(2, 1),
+ /*ExpansionLocEnd=*/Loc(2, 6), /*Length=*/4);
+ ASSERT_TRUE(MacroLoc.isMacroID());
+ std::optional<CharSourceRange> MacroR = getExpansionRangeInFile(
+ CharSourceRange::getTokenRange(MacroLoc, MacroLoc), FID, SM);
+ ASSERT_TRUE(MacroR);
+ EXPECT_EQ(SM.getFileID(MacroR->getBegin()), FID);
+ EXPECT_EQ(SM.getFileID(MacroR->getEnd()), FID);
+ EXPECT_EQ(SM.getFileOffset(MacroR->getBegin()),
+ SM.getFileOffset(Loc(2, 1)));
+ EXPECT_EQ(SM.getFileOffset(MacroR->getEnd()), SM.getFileOffset(Loc(2, 6)));
+}
+
} // anonymous namespace
More information about the llvm-branch-commits
mailing list