[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 07:40:37 PDT 2026
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/214461
>From 55220346717ab93d4fd39b5178d00a7640ee6429 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 | 5 +-
clang/lib/Frontend/DiagnosticRenderer.cpp | 15 ++--
.../unittests/Frontend/TextDiagnosticTest.cpp | 89 +++++++++++++++++++
3 files changed, 102 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/Frontend/DiagnosticRenderer.h b/clang/include/clang/Frontend/DiagnosticRenderer.h
index f5418dbdd8c09..622c068b8e61f 100644
--- a/clang/include/clang/Frontend/DiagnosticRenderer.h
+++ b/clang/include/clang/Frontend/DiagnosticRenderer.h
@@ -34,8 +34,9 @@ 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 nullopt if \p Range is invalid, if an endpoint lies outside \p FID,
+/// or if the beginning of the range is after the end.
///
/// Unlike \c 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..67760e2b7c4dd 100644
--- a/clang/lib/Frontend/DiagnosticRenderer.cpp
+++ b/clang/lib/Frontend/DiagnosticRenderer.cpp
@@ -40,14 +40,19 @@ 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..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