From 57dd8a94c08351605823acfe874f94819d19bad0 Mon Sep 17 00:00:00 2001 From: Yu Hao Date: Wed, 22 Oct 2025 15:00:12 -0700 Subject: [PATCH 1/3] [clang][transformer] Change `name` range-selector to return `Error` instead of an invalid range. Previously, when the text in selected range was different from the decl's name, `name` returned an invalid range, which could cause crashes if `name` was nested in other range selectors that assumed always valid ranges. With this change, `name` returns an `Error` if it can't get the range. --- .../lib/Tooling/Transformer/RangeSelector.cpp | 8 ++++-- clang/unittests/Tooling/RangeSelectorTest.cpp | 25 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/clang/lib/Tooling/Transformer/RangeSelector.cpp b/clang/lib/Tooling/Transformer/RangeSelector.cpp index 171c786bc366f..b4bdec1fcdd69 100644 --- a/clang/lib/Tooling/Transformer/RangeSelector.cpp +++ b/clang/lib/Tooling/Transformer/RangeSelector.cpp @@ -205,8 +205,12 @@ RangeSelector transformer::name(std::string ID) { // `foo` for which this range will be too short. Doing so will // require subcasing `NamedDecl`, because it doesn't provide virtual // access to the \c DeclarationNameInfo. - if (tooling::getText(R, *Result.Context) != D->getName()) - return CharSourceRange(); + StringRef Text = tooling::getText(R, *Result.Context); + if (Text != D->getName()) + return llvm::make_error( + llvm::errc::not_supported, + "range selected by name(node id=" + ID + "): '" + Text + + "' is different from decl name '" + D->getName() + "'"); return R; } if (const auto *E = Node.get()) { diff --git a/clang/unittests/Tooling/RangeSelectorTest.cpp b/clang/unittests/Tooling/RangeSelectorTest.cpp index adf5e74ea3192..a1fcbb023832f 100644 --- a/clang/unittests/Tooling/RangeSelectorTest.cpp +++ b/clang/unittests/Tooling/RangeSelectorTest.cpp @@ -527,6 +527,31 @@ TEST(RangeSelectorTest, NameOpDeclRefError) { AllOf(HasSubstr(Ref), HasSubstr("requires property 'identifier'"))))); } +TEST(RangeSelectorTest, NameOpDeclInMacroArg) { + StringRef Code = R"cc( + #define MACRO(name) int name; + MACRO(x) + )cc"; + const char *ID = "id"; + TestMatch Match = matchCode(Code, varDecl().bind(ID)); + EXPECT_THAT_EXPECTED(select(name(ID), Match), HasValue("x")); +} + +TEST(RangeSelectorTest, NameOpDeclInMacroBodyError) { + StringRef Code = R"cc( + #define MACRO int x; + MACRO + )cc"; + const char *ID = "id"; + TestMatch Match = matchCode(Code, varDecl().bind(ID)); + EXPECT_THAT_EXPECTED( + name(ID)(Match.Result), + Failed(testing::Property( + &StringError::getMessage, + AllOf(HasSubstr("range selected by name(node id="), + HasSubstr("' is different from decl name 'x'"))))); +} + TEST(RangeSelectorTest, CallArgsOp) { const StringRef Code = R"cc( struct C { From ee0eaed70a3cfbf0306cc772be36361ee0da6fa0 Mon Sep 17 00:00:00 2001 From: Yu Hao Date: Tue, 11 Nov 2025 17:41:29 -0800 Subject: [PATCH 2/3] [clang][transformer] Fix `node` range-selector to include type name qualifiers of type locs. Previously, e.g. for TypeLoc "MyNamespace::MyClass", `node()` selects only "MyClass" without the qualifier. With this change, it now selects "MyNamespace::MyClass". --- clang/lib/Tooling/Transformer/RangeSelector.cpp | 2 +- clang/unittests/Tooling/RangeSelectorTest.cpp | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/clang/lib/Tooling/Transformer/RangeSelector.cpp b/clang/lib/Tooling/Transformer/RangeSelector.cpp index b4bdec1fcdd69..d669be2ca0409 100644 --- a/clang/lib/Tooling/Transformer/RangeSelector.cpp +++ b/clang/lib/Tooling/Transformer/RangeSelector.cpp @@ -139,7 +139,7 @@ RangeSelector transformer::node(std::string ID) { (Node->get() != nullptr && Node->get() == nullptr)) ? tooling::getExtendedRange(*Node, tok::TokenKind::semi, *Result.Context) - : CharSourceRange::getTokenRange(Node->getSourceRange()); + : CharSourceRange::getTokenRange(Node->getSourceRange(true)); }; } diff --git a/clang/unittests/Tooling/RangeSelectorTest.cpp b/clang/unittests/Tooling/RangeSelectorTest.cpp index a1fcbb023832f..d441da165b09b 100644 --- a/clang/unittests/Tooling/RangeSelectorTest.cpp +++ b/clang/unittests/Tooling/RangeSelectorTest.cpp @@ -339,6 +339,13 @@ TEST(RangeSelectorTest, NodeOpExpression) { EXPECT_THAT_EXPECTED(select(node("id"), Match), HasValue("3")); } +TEST(RangeSelectorTest, NodeOpTypeLoc) { + StringRef Code = "namespace ns {struct Foo{};} ns::Foo a;"; + TestMatch Match = + matchCode(Code, varDecl(hasTypeLoc(typeLoc().bind("typeloc")))); + EXPECT_THAT_EXPECTED(select(node("typeloc"), Match), HasValue("ns::Foo")); +} + TEST(RangeSelectorTest, StatementOp) { StringRef Code = "int f() { return 3; }"; TestMatch Match = matchCode(Code, expr().bind("id")); From 1a1de248402f7c6ea79ba0907ada695efe23f166 Mon Sep 17 00:00:00 2001 From: Yu Hao Date: Tue, 18 Nov 2025 14:35:02 -0800 Subject: [PATCH 3/3] add comment for the argument --- clang/lib/Tooling/Transformer/RangeSelector.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Tooling/Transformer/RangeSelector.cpp b/clang/lib/Tooling/Transformer/RangeSelector.cpp index d669be2ca0409..54a1590d3106d 100644 --- a/clang/lib/Tooling/Transformer/RangeSelector.cpp +++ b/clang/lib/Tooling/Transformer/RangeSelector.cpp @@ -139,7 +139,8 @@ RangeSelector transformer::node(std::string ID) { (Node->get() != nullptr && Node->get() == nullptr)) ? tooling::getExtendedRange(*Node, tok::TokenKind::semi, *Result.Context) - : CharSourceRange::getTokenRange(Node->getSourceRange(true)); + : CharSourceRange::getTokenRange( + Node->getSourceRange(/*IncludeQualifier=*/true)); }; }