[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