Skip to content

[ASTMatchers] Extend hasName matcher when matching templates #100349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[ASTMatchers] Extend hasName matcher when matching templates #100349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions clang/include/clang/ASTMatchers/ASTMatchers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3076,6 +3076,21 @@ inline internal::BindableMatcher<Stmt> sizeOfExpr(
/// \code
/// namespace a { namespace b { class X; } }
/// \endcode
///
/// Qualified names in templated classes can be matched explicitly or implicity
/// by specifying the template type or using `<*>` to match any template.
Copy link
Collaborator

@AaronBallman AaronBallman Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably be clear that <*> is the only syntax we support. e.g., we don't let you search based on arity, as in hasName("Foo<int, *>::Bar"). Might be worth an example as well, definitely worth some test coverage.
///
/// Example matches:
/// - callExpr(callee(functionDecl(hasName("Foo<int>::Bar"))))
/// - callExpr(callee(functionDecl(hasName("Foo<>::Bar"))))
/// \code
/// template<typename T> class Foo{
/// static void Bar();
/// };
/// void Func() {
/// Foo<int>::Bar();
/// }
/// \endcode
inline internal::Matcher<NamedDecl> hasName(StringRef Name) {
return internal::Matcher<NamedDecl>(
new internal::HasNameMatcher({std::string(Name)}));
Expand Down
41 changes: 38 additions & 3 deletions clang/lib/ASTMatchers/ASTMatchersInternal.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,39 @@ bool HasNameMatcher::matchesNodeFullFast(const NamedDecl &Node) const {
return Patterns.foundMatch(/*AllowFullyQualified=*/true);
}

static std::optional<StringRef> consumePatternBack(StringRef Pattern,
StringRef Target) {
while (!Pattern.empty()) {
auto Index = Pattern.rfind("<*>");
if (Index == StringRef::npos) {
if (Target.consume_back(Pattern))
return Target;
return {};
}
auto Suffix = Pattern.substr(Index + 2);
if (!Target.consume_back(Suffix))
Comment on lines +650 to +651
Copy link
Contributor

@5chmidti 5chmidti Aug 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I had to check this, could you please add a comment why only + 2 is used?
Something like:

We consume pattern suffix that includes only the > of our <*>, and drop every character from the back until we encounter the corresponding < character to ensure balanced angle brackets.
return {};
auto BracketCount = 1;
Comment on lines +644 to +653
Copy link
Contributor

@5chmidti 5chmidti Aug 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're using auto three times here without the type mentioned on the RHS (not sure how strict clang/ is w.r.t. this)

Suffix and Index can be const
while (BracketCount) {
if (Target.empty())
return {};
Comment on lines +648 to +656
Copy link
Contributor

@5chmidti 5chmidti Aug 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO returning std::nullopt is better here.
switch (Target.back()) {
case '<':
--BracketCount;
break;
case '>':
++BracketCount;
break;
default:
break;
}
Target = Target.drop_back();
}
Pattern = Pattern.take_front(Index);
}
return Target;
}

bool HasNameMatcher::matchesNodeFullSlow(const NamedDecl &Node) const {
const bool SkipUnwrittenCases[] = {false, true};
for (bool SkipUnwritten : SkipUnwrittenCases) {
Expand All @@ -653,10 +686,12 @@ bool HasNameMatcher::matchesNodeFullSlow(const NamedDecl &Node) const {

for (const StringRef Pattern : Names) {
if (Pattern.starts_with("::")) {
if (FullName == Pattern)
if (auto Result = consumePatternBack(Pattern, FullName);
Result && Result->empty()) {
return true;
} else if (FullName.ends_with(Pattern) &&
FullName.drop_back(Pattern.size()).ends_with("::")) {
}
} else if (auto Result = consumePatternBack(Pattern, FullName);
Result && Result->ends_with("::")) {
return true;
}
Comment on lines 688 to 696
Copy link
Contributor

@5chmidti 5chmidti Aug 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could write this loop body as

 const std::optional<StringRef> Result = consumePatternBack(Pattern, FullName); if (!Result) continue; if ((Pattern.starts_with("::") && Result->empty()) || Result->ends_with("::")) return true;

which is more readable IMO
}
Expand Down
35 changes: 35 additions & 0 deletions clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2664,6 +2664,41 @@ TEST_P(ASTMatchersTest, HasName_QualifiedStringMatchesThroughLinkage) {
EXPECT_TRUE(notMatches(code, functionDecl(hasName("::test"))));
}

TEST_P(ASTMatchersTest, HasName_TemplateStrip) {
if (!GetParam().isCXX()) {
return;
}

StringRef Code = "template<typename T> struct Foo{void Bar() const;}; void "
"Func() { Foo<int> Item; Item.Bar(); }";

EXPECT_TRUE(matches(Code, callExpr(callee(cxxMethodDecl(hasName("Bar"))))));
EXPECT_TRUE(
matches(Code, callExpr(callee(cxxMethodDecl(hasName("Foo<int>::Bar"))))));
EXPECT_TRUE(matches(
Code, callExpr(callee(cxxMethodDecl(hasName("::Foo<int>::Bar"))))));
EXPECT_TRUE(
matches(Code, callExpr(callee(cxxMethodDecl(hasName("::Foo<*>::Bar"))))));
EXPECT_TRUE(notMatches(
Code, callExpr(callee(cxxMethodDecl(hasName("::Foo<::Bar"))))));
EXPECT_TRUE(notMatches(
Code, callExpr(callee(cxxMethodDecl(hasName("::Foo<long>::Bar"))))));

if (GetParam().isCXX11OrLater()) {
Code = "template<typename T> struct Foo{void Bar() const;}; void Func() { "
"Foo<Foo<int>> Item; Item.Bar(); }";
EXPECT_TRUE(matches(Code, callExpr(callee(cxxMethodDecl(hasName("Bar"))))));
EXPECT_TRUE(
matches(Code, callExpr(callee(cxxMethodDecl(hasName("Foo<*>::Bar"))))));
EXPECT_TRUE(matches(
Code, callExpr(callee(cxxMethodDecl(hasName("::Foo<*>::Bar"))))));
EXPECT_TRUE(matches(
Code, callExpr(callee(cxxMethodDecl(hasName("Foo<Foo<*>>::Bar"))))));
EXPECT_TRUE(matches(
Code, callExpr(callee(cxxMethodDecl(hasName("::Foo<Foo<*>>::Bar"))))));
}
}

TEST_P(ASTMatchersTest, HasAnyName) {
if (!GetParam().isCXX()) {
// FIXME: Add a test for `hasAnyName()` that does not depend on C++.
Expand Down