[Clang][ASTMatcher] Improve matching isDerivedFrom base in case of multi aliases exists

[AmrDeveloper]

Previously in case of multiable aliases exists for the same base we just accept the first one

llvm-project/clang/lib/ASTMatchers/ASTMatchFinder.cpp

Lines 1290 to 1297 in 2cf6663

	for (const TypedefNameDecl *Alias : Aliases->second) {
	BoundNodesTreeBuilder Result(*Builder);
	if (Matcher.matches(*Alias, this, &Result)) {
	*Builder = std::move(Result);
	return true;
	}
	}
	return false;

For example

struct AnInterface {};
typedef AnInterface UnusedTypedef;
typedef AnInterface Base;
class AClass : public Base {};

cxxRecordDecl(isDerivedFrom(decl().bind("typedef"))) typedef will be bonded to UnusedTypedef

But if we changed the order now it will point to the right one

struct AnInterface {};
typedef AnInterface Base;
typedef AnInterface UnusedTypedef;
class AClass : public Base {};

cxxRecordDecl(isDerivedFrom(decl().bind("typedef"))) typedef will be bonded to Base

This PR improve the matcher to prioritise the alias that has same desugared name as the base, if not then just accept the first one.

Fixes: #126498

[llvmbot]

@llvm/pr-subscribers-clang

Author: Amr Hesham (AmrDeveloper)

Changes

Previously in case of multiable aliases exists for the same base we just accept the first one

llvm-project/clang/lib/ASTMatchers/ASTMatchFinder.cpp

Lines 1290 to 1297 in 2cf6663

	for (const TypedefNameDecl *Alias : Aliases->second) {
	BoundNodesTreeBuilder Result(*Builder);
	if (Matcher.matches(*Alias, this, &Result)) {
	*Builder = std::move(Result);
	return true;
	}
	}
	return false;

For example

struct AnInterface {};
typedef AnInterface UnusedTypedef;
typedef AnInterface Base;
class AClass : public Base {};

cxxRecordDecl(isDerivedFrom(decl().bind("typedef"))) typedef will be bonded to UnusedTypedef

But if we changed the order now it will point to the right one

struct AnInterface {};
typedef AnInterface Base;
typedef AnInterface UnusedTypedef;
class AClass : public Base {};

cxxRecordDecl(isDerivedFrom(decl().bind("typedef"))) typedef will be bonded to Base

This PR improve the matcher to prioritise the alias that has same desugared name as the base, if not then just accept the first one.

Fixes: #126498

---

Full diff: https://github.com/llvm/llvm-project/pull/126793.diff

2 Files Affected:

• (modified) clang/lib/ASTMatchers/ASTMatchFinder.cpp (+21)
• (modified) clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp (+17)

diff --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
index 3d01a70395a9b..e9ec7eff1e0ab 100644
--- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -1287,6 +1287,27 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
     auto Aliases = TypeAliases.find(CanonicalType);
     if (Aliases == TypeAliases.end())
       return false;
+
+    if (const auto *ElaboratedTypeNode =
+            llvm::dyn_cast<ElaboratedType>(TypeNode)) {
+      if (ElaboratedTypeNode->isSugared() && Aliases->second.size() > 1) {
+        const auto &DesugaredTypeName =
+            ElaboratedTypeNode->desugar().getAsString();
+
+        for (const TypedefNameDecl *Alias : Aliases->second) {
+          if (Alias->getName() != DesugaredTypeName) {
+            continue;
+          }
+
+          BoundNodesTreeBuilder Result(*Builder);
+          if (Matcher.matches(*Alias, this, &Result)) {
+            *Builder = std::move(Result);
+            return true;
+          }
+        }
+      }
+    }
+
     for (const TypedefNameDecl *Alias : Aliases->second) {
       BoundNodesTreeBuilder Result(*Builder);
       if (Matcher.matches(*Alias, this, &Result)) {
diff --git a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
index 5e1c12ba26d87..4e6baedae2be5 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1167,6 +1167,23 @@ TEST_P(ASTMatchersTest, IsDerivedFrom_EmptyName) {
   EXPECT_TRUE(notMatches(Code, cxxRecordDecl(isSameOrDerivedFrom(""))));
 }
 
+TEST_P(ASTMatchersTest, IsDerivedFrom_ElaboratedType) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+
+  DeclarationMatcher IsDerivenFromBase =
+      cxxRecordDecl(isDerivedFrom(decl().bind("typedef")));
+
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+      "struct AnInterface {};"
+      "typedef AnInterface UnusedTypedef;"
+      "typedef AnInterface Base;"
+      "class AClass : public Base {};",
+      IsDerivenFromBase,
+      std::make_unique<VerifyIdIsBoundTo<TypedefDecl>>("typedef", "Base")));
+}
+
 TEST_P(ASTMatchersTest, IsDerivedFrom_ObjC) {
   DeclarationMatcher IsDerivedFromX = objcInterfaceDecl(isDerivedFrom("X"));
   EXPECT_TRUE(

[HighCommander4]

This is a bit more in the weeds of AST matchers internals than what I'm familiar with, so I've added some other reviewers who have made or reviewed more extensive changes to this code based on code history.

[AmrDeveloper]

Please take a look when you have time @AaronBallman @alexfh @steveire

[AaronBallman]

LGTM, but you should add a release note to clang/docs/ReleaseNotes.rst so users know about the change in behavior.

[AmrDeveloper]

LGTM, but you should add a release note to clang/docs/ReleaseNotes.rst so users know about the change in behavior.

Thank you, I added a release note, once you confirm it's approved I can land after CI is green

[AmrDeveloper]

@AaronBallman If releasenote is okey, i can merge now :D

[AaronBallman]

Just a minor rewording, feel free to land once you've made the changes.

[AaronBallman]

Suggested change

	- Ensure ``isDerivedFrom`` is matching the correct base in case of more than one aliases exists.
	- Ensure ``isDerivedFrom`` matches the correct base in case more than one alias exists.

Minor rewording