Skip to content

[ASTMatchers] Fix classIsDerivedFrom for recusrive cases #67307

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

Merged
merged 2 commits into from
Sep 25, 2023
Merged

[ASTMatchers] Fix classIsDerivedFrom for recusrive cases #67307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd300e3
[ASTMatchers] Fix classIsDerivedFrom for recusrive cases
ilya-biryukov Sep 25, 2023
5e7adf9
fixup! [ASTMatchers] Fix classIsDerivedFrom for recusrive cases
ilya-biryukov Sep 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion clang/lib/ASTMatchers/ASTMatchFinder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/PrettyStackTrace.h"
#include "llvm/Support/Timer.h"
Expand Down Expand Up @@ -651,11 +653,20 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
BoundNodesTreeBuilder *Builder,
bool Directly) override;

private:
bool
classIsDerivedFromImpl(const CXXRecordDecl *Declaration,
const Matcher<NamedDecl> &Base,
BoundNodesTreeBuilder *Builder, bool Directly,
llvm::SmallPtrSetImpl<const CXXRecordDecl *> &Visited);

public:
bool objcClassIsDerivedFrom(const ObjCInterfaceDecl *Declaration,
const Matcher<NamedDecl> &Base,
BoundNodesTreeBuilder *Builder,
bool Directly) override;

public:
// Implements ASTMatchFinder::matchesChildOf.
bool matchesChildOf(const DynTypedNode &Node, ASTContext &Ctx,
const DynTypedMatcher &Matcher,
Expand Down Expand Up @@ -1361,8 +1372,18 @@ bool MatchASTVisitor::classIsDerivedFrom(const CXXRecordDecl *Declaration,
const Matcher<NamedDecl> &Base,
BoundNodesTreeBuilder *Builder,
bool Directly) {
llvm::SmallPtrSet<const CXXRecordDecl *, 8> Visited;
return classIsDerivedFromImpl(Declaration, Base, Builder, Directly, Visited);
}

bool MatchASTVisitor::classIsDerivedFromImpl(
const CXXRecordDecl *Declaration, const Matcher<NamedDecl> &Base,
BoundNodesTreeBuilder *Builder, bool Directly,
llvm::SmallPtrSetImpl<const CXXRecordDecl *> &Visited) {
if (!Declaration->hasDefinition())
return false;
if (!Visited.insert(Declaration).second)
return false;
for (const auto &It : Declaration->bases()) {
const Type *TypeNode = It.getType().getTypePtr();

Expand All @@ -1384,7 +1405,8 @@ bool MatchASTVisitor::classIsDerivedFrom(const CXXRecordDecl *Declaration,
*Builder = std::move(Result);
return true;
}
if (!Directly && classIsDerivedFrom(ClassDecl, Base, Builder, Directly))
if (!Directly &&
classIsDerivedFromImpl(ClassDecl, Base, Builder, Directly, Visited))
return true;
}
return false;
Expand Down
74 changes: 74 additions & 0 deletions clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2369,6 +2369,80 @@ TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureOfReferenceType) {
"}", matcher));
}

TEST_P(ASTMatchersTest, IsDerivedFromRecursion) {
if (!GetParam().isCXX11OrLater())
return;

// Check we don't crash on cycles in the traversal and inheritance hierarchy.
// Clang will normally enforce there are no cycles, but matchers opted to
// traverse primary template for dependent specializations, spuriously
// creating the cycles.
DeclarationMatcher matcher = cxxRecordDecl(isDerivedFrom("X"));
EXPECT_TRUE(notMatches(R"cpp(
template <typename T1, typename T2>
struct M;

template <typename T1>
struct M<T1, void> {};

template <typename T1, typename T2>
struct L : M<T1, T2> {};

template <typename T1, typename T2>
struct M : L<M<T1, T2>, M<T1, T2>> {};
)cpp",
matcher));

// Check the running time is not exponential. The number of subojects to
// traverse grows as fibonacci numbers even though the number of bases to
// traverse is quadratic.
// The test will hang if implementation of matchers traverses all subojects.
EXPECT_TRUE(notMatches(R"cpp(
template <class T> struct A0 {};
template <class T> struct A1 : A0<T> {};
template <class T> struct A2 : A1<T>, A0<T> {};
template <class T> struct A3 : A2<T>, A1<T> {};
template <class T> struct A4 : A3<T>, A2<T> {};
template <class T> struct A5 : A4<T>, A3<T> {};
template <class T> struct A6 : A5<T>, A4<T> {};
template <class T> struct A7 : A6<T>, A5<T> {};
template <class T> struct A8 : A7<T>, A6<T> {};
template <class T> struct A9 : A8<T>, A7<T> {};
template <class T> struct A10 : A9<T>, A8<T> {};
template <class T> struct A11 : A10<T>, A9<T> {};
template <class T> struct A12 : A11<T>, A10<T> {};
template <class T> struct A13 : A12<T>, A11<T> {};
template <class T> struct A14 : A13<T>, A12<T> {};
template <class T> struct A15 : A14<T>, A13<T> {};
template <class T> struct A16 : A15<T>, A14<T> {};
template <class T> struct A17 : A16<T>, A15<T> {};
template <class T> struct A18 : A17<T>, A16<T> {};
template <class T> struct A19 : A18<T>, A17<T> {};
template <class T> struct A20 : A19<T>, A18<T> {};
template <class T> struct A21 : A20<T>, A19<T> {};
template <class T> struct A22 : A21<T>, A20<T> {};
template <class T> struct A23 : A22<T>, A21<T> {};
template <class T> struct A24 : A23<T>, A22<T> {};
template <class T> struct A25 : A24<T>, A23<T> {};
template <class T> struct A26 : A25<T>, A24<T> {};
template <class T> struct A27 : A26<T>, A25<T> {};
template <class T> struct A28 : A27<T>, A26<T> {};
template <class T> struct A29 : A28<T>, A27<T> {};
template <class T> struct A30 : A29<T>, A28<T> {};
template <class T> struct A31 : A30<T>, A29<T> {};
template <class T> struct A32 : A31<T>, A30<T> {};
template <class T> struct A33 : A32<T>, A31<T> {};
template <class T> struct A34 : A33<T>, A32<T> {};
template <class T> struct A35 : A34<T>, A33<T> {};
template <class T> struct A36 : A35<T>, A34<T> {};
template <class T> struct A37 : A36<T>, A35<T> {};
template <class T> struct A38 : A37<T>, A36<T> {};
template <class T> struct A39 : A38<T>, A37<T> {};
template <class T> struct A40 : A39<T>, A38<T> {};
)cpp",
matcher));
}

TEST(ASTMatchersTestObjC, ObjCMessageCalees) {
StatementMatcher MessagingFoo =
objcMessageExpr(callee(objcMethodDecl(hasName("foo"))));
Expand Down