[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 18:45:27 PDT 2024


danix800 wrote:

> I think it would be a good idea to double check this for performance regressions, since this case will recurse into the function every time with this patch. Though I don't know if there is a better way to test it than to merge it and wait for it to be picked up by google.

For large TU it's worth considering for performance. The previous commit relaxed the `TypeVisitor` checking to all `FunctionDecl`. I reverted back to the original `auto` impl, plus an extra condition that if it's a `lambda` definition, see https://github.com/llvm/llvm-project/pull/89096/commits/3f1d714a2aa4b236afe9bb1384be073fb155a2b8

> 
> Another idea to avoid any potential regressions, is to mark the return type as deduced anyway, with the distinction that 'auto' was not written.

I tested this idea, might be like this:
```
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 712958594473..988efcbef3a3 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -3680,24 +3680,10 @@ 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)) {
-      FromReturnTy = Importer.getFromContext().VoidTy;
-      UsedDifferentProtoType = true;
-    }
-    FunctionProtoType::ExtProtoInfo FromEPI = FromFPT->getExtProtoInfo();
-    // FunctionProtoType::ExtProtoInfo's ExceptionSpecDecl can point to the
-    // FunctionDecl that we are importing the FunctionProtoType for.
-    // To avoid an infinite recursion when importing, create the FunctionDecl
-    // with a simplified function type.
-    if (FromEPI.ExceptionSpec.SourceDecl ||
-        FromEPI.ExceptionSpec.SourceTemplate ||
-        FromEPI.ExceptionSpec.NoexceptExpr) {
-      FunctionProtoType::ExtProtoInfo DefaultEPI;
-      FromEPI = DefaultEPI;
-      UsedDifferentProtoType = true;
-    }
+    FunctionProtoType::ExtProtoInfo DefaultEPI;
+    UsedDifferentProtoType = true;
     FromTy = Importer.getFromContext().getFunctionType(
-        FromReturnTy, FromFPT->getParamTypes(), FromEPI);
+        Importer.getFromContext().VoidTy, FromFPT->getParamTypes(), DefaultEPI);
     FromTSI = Importer.getFromContext().getTrivialTypeSourceInfo(
         FromTy, D->getBeginLoc());
   }
```
but this will break two testcases from `check-clang-astmerge-exprs-cpp`:
```
Failed Tests (2):
  Clang :: ASTMerge/class-template/test.cpp
  Clang :: ASTMerge/exprs-cpp/test.cpp
```

https://github.com/llvm/llvm-project/pull/89096


More information about the cfe-commits mailing list