[clang] [ASTImporter] Fix infinite recurse on return type declared inside body (#68775) (PR #89096)
Ding Fei via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 17 22:18:51 PDT 2024
https://github.com/danix800 updated https://github.com/llvm/llvm-project/pull/89096
>From 0d6d52365a5d31045c347412c3a0fe8be7119006 Mon Sep 17 00:00:00 2001
From: dingfei <fding at feysh.com>
Date: Thu, 18 Apr 2024 00:33:29 +0800
Subject: [PATCH 1/4] [ASTImporter] Fix infinite recurse on return type
declared inside body (#68775)
Lambda without trailing auto could has return type declared
inside the body too.
---
clang/docs/ReleaseNotes.rst | 2 ++
clang/lib/AST/ASTImporter.cpp | 16 ++++++----------
clang/unittests/AST/ASTImporterTest.cpp | 16 ++++++++++++++++
3 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c19ad9fba58f37..7a623c6be72fd2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -545,6 +545,8 @@ Bug Fixes to AST Handling
Miscellaneous Bug Fixes
^^^^^^^^^^^^^^^^^^^^^^^
+- Fixed an infinite recursion on return type declared inside body (#GH68775).
+
Miscellaneous Clang Crashes Fixed
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 6aaa34c55ce307..88883bbd6ebaf1 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -695,7 +695,7 @@ namespace clang {
// Returns true if the given function has a placeholder return type and
// that type is declared inside the body of the function.
// E.g. auto f() { struct X{}; return X(); }
- bool hasAutoReturnTypeDeclaredInside(FunctionDecl *D);
+ bool hasReturnTypeDeclaredInside(FunctionDecl *D);
};
template <typename InContainerTy>
@@ -3649,19 +3649,15 @@ class IsTypeDeclaredInsideVisitor
/// This function checks if the function has 'auto' return type that contains
/// a reference (in any way) to a declaration inside the same function.
-bool ASTNodeImporter::hasAutoReturnTypeDeclaredInside(FunctionDecl *D) {
+bool ASTNodeImporter::hasReturnTypeDeclaredInside(FunctionDecl *D) {
QualType FromTy = D->getType();
const auto *FromFPT = FromTy->getAs<FunctionProtoType>();
assert(FromFPT && "Must be called on FunctionProtoType");
QualType RetT = FromFPT->getReturnType();
- if (isa<AutoType>(RetT.getTypePtr())) {
- FunctionDecl *Def = D->getDefinition();
- IsTypeDeclaredInsideVisitor Visitor(Def ? Def : D);
- return Visitor.CheckType(RetT);
- }
-
- return false;
+ FunctionDecl *Def = D->getDefinition();
+ IsTypeDeclaredInsideVisitor Visitor(Def ? Def : D);
+ return Visitor.CheckType(RetT);
}
ExplicitSpecifier
@@ -3811,7 +3807,7 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
// E.g.: auto foo() { struct X{}; return X(); }
// To avoid an infinite recursion when importing, create the FunctionDecl
// with a simplified return type.
- if (hasAutoReturnTypeDeclaredInside(D)) {
+ if (hasReturnTypeDeclaredInside(D)) {
FromReturnTy = Importer.getFromContext().VoidTy;
UsedDifferentProtoType = true;
}
diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index acc596fef87b76..3fac5514319c24 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -6721,6 +6721,22 @@ TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionBody) {
EXPECT_FALSE(FromL->isDependentLambda());
}
+TEST_P(ASTImporterOptionSpecificTestBase, LambdaReturnTypeDeclaredInside) {
+ Decl *From, *To;
+ std::tie(From, To) = getImportedDecl(
+ R"(
+ void foo() {
+ (void) []() {
+ struct X {};
+ return X();
+ };
+ }
+ )",
+ Lang_CXX11, "", Lang_CXX11, "foo");
+ auto *ToLambda = FirstDeclMatcher<LambdaExpr>().match(To, lambdaExpr());
+ EXPECT_TRUE(ToLambda);
+}
+
TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionParam) {
Decl *FromTU = getTuDecl(
R"(
>From 3f1d714a2aa4b236afe9bb1384be073fb155a2b8 Mon Sep 17 00:00:00 2001
From: dingfei <fding at feysh.com>
Date: Thu, 18 Apr 2024 08:56:56 +0800
Subject: [PATCH 2/4] Limit TypeVisitor checking to 'auto' and lambda
definition only
1. Fix potential perf regression;
2. Fix comment.
---
clang/lib/AST/ASTImporter.cpp | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 88883bbd6ebaf1..d98cb80c349db2 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -3647,17 +3647,25 @@ class IsTypeDeclaredInsideVisitor
};
} // namespace
-/// This function checks if the function has 'auto' return type that contains
+/// This function checks if the given function has a return type that contains
/// a reference (in any way) to a declaration inside the same function.
bool ASTNodeImporter::hasReturnTypeDeclaredInside(FunctionDecl *D) {
QualType FromTy = D->getType();
const auto *FromFPT = FromTy->getAs<FunctionProtoType>();
assert(FromFPT && "Must be called on FunctionProtoType");
+ bool IsLambdaDefinition = false;
+ if (const auto *MD = dyn_cast<CXXMethodDecl>(D))
+ IsLambdaDefinition = cast<CXXRecordDecl>(MD->getDeclContext())->isLambda();
+
QualType RetT = FromFPT->getReturnType();
- FunctionDecl *Def = D->getDefinition();
- IsTypeDeclaredInsideVisitor Visitor(Def ? Def : D);
- return Visitor.CheckType(RetT);
+ if (isa<AutoType>(RetT.getTypePtr()) || IsLambdaDefinition) {
+ FunctionDecl *Def = D->getDefinition();
+ IsTypeDeclaredInsideVisitor Visitor(Def ? Def : D);
+ return Visitor.CheckType(RetT);
+ }
+
+ return false;
}
ExplicitSpecifier
>From d516938f47e782517f1c1e126b8fa5a98398a112 Mon Sep 17 00:00:00 2001
From: dingfei <fding at feysh.com>
Date: Thu, 18 Apr 2024 13:12:19 +0800
Subject: [PATCH 3/4] Check for trailing return and limit to c++11 only
The checking is refactored into a call so that if 'auto' presents, the checking
is short-circuited.
---
clang/lib/AST/ASTImporter.cpp | 18 ++++++++++++++----
clang/unittests/AST/ASTImporterTest.cpp | 5 +++--
2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index d98cb80c349db2..79df4f74dd5cbb 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -3522,6 +3522,7 @@ class IsTypeDeclaredInsideVisitor
: ParentDC(ParentDC) {}
bool CheckType(QualType T) {
+ T->isUndeducedType();
// Check the chain of "sugar" types.
// The "sugar" types are typedef or similar types that have the same
// canonical type.
@@ -3654,12 +3655,21 @@ bool ASTNodeImporter::hasReturnTypeDeclaredInside(FunctionDecl *D) {
const auto *FromFPT = FromTy->getAs<FunctionProtoType>();
assert(FromFPT && "Must be called on FunctionProtoType");
- bool IsLambdaDefinition = false;
- if (const auto *MD = dyn_cast<CXXMethodDecl>(D))
- IsLambdaDefinition = cast<CXXRecordDecl>(MD->getDeclContext())->isLambda();
+ auto IsCXX11LambdaWithouTrailingReturn = [&]() {
+ if (!Importer.FromContext.getLangOpts().CPlusPlus11)
+ return false;
+
+ if (FromFPT->hasTrailingReturn())
+ return false;
+
+ if (const auto *MD = dyn_cast<CXXMethodDecl>(D))
+ return cast<CXXRecordDecl>(MD->getDeclContext())->isLambda();
+
+ return false;
+ };
QualType RetT = FromFPT->getReturnType();
- if (isa<AutoType>(RetT.getTypePtr()) || IsLambdaDefinition) {
+ if (isa<AutoType>(RetT.getTypePtr()) || IsCXX11LambdaWithouTrailingReturn()) {
FunctionDecl *Def = D->getDefinition();
IsTypeDeclaredInsideVisitor Visitor(Def ? Def : D);
return Visitor.CheckType(RetT);
diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index 3fac5514319c24..4ee64de697d37a 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -6721,7 +6721,8 @@ TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionBody) {
EXPECT_FALSE(FromL->isDependentLambda());
}
-TEST_P(ASTImporterOptionSpecificTestBase, LambdaReturnTypeDeclaredInside) {
+TEST_P(ASTImporterOptionSpecificTestBase,
+ ReturnTypeDeclaredInsideOfCXX11LambdaWithoutTrailingReturn) {
Decl *From, *To;
std::tie(From, To) = getImportedDecl(
R"(
@@ -6732,7 +6733,7 @@ TEST_P(ASTImporterOptionSpecificTestBase, LambdaReturnTypeDeclaredInside) {
};
}
)",
- Lang_CXX11, "", Lang_CXX11, "foo");
+ Lang_CXX11, "", Lang_CXX11, "foo"); // c++11 only
auto *ToLambda = FirstDeclMatcher<LambdaExpr>().match(To, lambdaExpr());
EXPECT_TRUE(ToLambda);
}
>From 7ae75e28ad8340c9867d6ee93550abe67affdd8e Mon Sep 17 00:00:00 2001
From: dingfei <fding at feysh.com>
Date: Thu, 18 Apr 2024 13:18:21 +0800
Subject: [PATCH 4/4] Update clang ReleaseNotes.rst
---
clang/docs/ReleaseNotes.rst | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7a623c6be72fd2..a528b0984cd1af 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -545,7 +545,8 @@ Bug Fixes to AST Handling
Miscellaneous Bug Fixes
^^^^^^^^^^^^^^^^^^^^^^^
-- Fixed an infinite recursion on return type declared inside body (#GH68775).
+- Fixed an infinite recursion in ASTImporter, on return type declared inside
+ body of C++11 lambda without trailing return (#GH68775).
Miscellaneous Clang Crashes Fixed
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
More information about the cfe-commits
mailing list