martong added a comment. Thanks, nice work!
================ Comment at: clang/lib/AST/ASTImporter.cpp:3237 + ParentMapContext &ParentC = DC->getParentASTContext().getParentMapContext(); + DynTypedNodeList P = ParentC.getParents(*S); + while (!P.empty()) { ---------------- ================ Comment at: clang/lib/AST/ASTImporter.cpp:3237 + ParentMapContext &ParentC = DC->getParentASTContext().getParentMapContext(); + DynTypedNodeList P = ParentC.getParents(*S); + while (!P.empty()) { ---------------- martong wrote: > The first call of `getParents` will create the parent map, via a full-blown AST visitation. I am concerned a bit about the additional performance overhead. Could you please run some measurements? (E.g. a CTU run on `protobuf` and `bitcoin` with our internal CI infra) ================ Comment at: clang/lib/AST/ASTImporter.cpp:3251-3254 + if (Arg.getKind() == TemplateArgument::Type) + return hasTypeDeclaredInsideFunction(Arg.getAsType(), FD); + if (Arg.getKind() == TemplateArgument::Expression) + return isAncestorDeclContextOf(FD, Arg.getAsExpr()); ---------------- Should this be handled in a `switch` rather, perhaps with an `llvm_unreachable` at the `default` case? Just to make sure that no "kind" is left out. ================ Comment at: clang/lib/AST/ASTImporter.cpp:3269-3273 + // Note: It is possible that T can be get as both a RecordType and a + // TemplateSpecializationType. + } + if (const auto *TST = T->getAs<TemplateSpecializationType>()) { + return llvm::count_if(TST->template_arguments(), CheckTemplateArgument); ---------------- Is it possible that `T` is both a `RecordType` and a `TemplateSpecializationType` at the same time? From the [[ https://clang.llvm.org/doxygen/classclang_1_1TemplateSpecializationType.html | hierarchy ]] this seems impossible (?) ================ Comment at: clang/unittests/AST/ASTImporterTest.cpp:6323 +TEST_P(ImportAutoFunctions, ReturnWithTemplateWithIntegerArgDeclaredInside) { + Decl *FromTU = getTuDecl( ---------------- Nice test! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129640/new/ https://reviews.llvm.org/D129640 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits