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
