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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
