[ASTImporter] Support importing FunctionTemplateDecl and CXXDependentScopeMemberExpr

[szepet]

Summary

This patch adds support for importing two different kind of AST nodes (FunctionTemplateDecl and CXXDependentScopeMemberExpr).
Note: This solution is based on D26904 (which I did not wanted to commandeer right away) which is probably based on (https://github.com/haoNoQ/clang/blob/summary-ipa-draft/lib/AST/ASTImporter.cpp#L7564).

[xazax.hun]

lib/AST/ASTImporter.cpp

4281

Do you need to set the access here? Isn't there some code that is triggered earlier and set this?

[szepet]

lib/AST/ASTImporter.cpp

4281

Unfortunately no, it has to be set manually.

[a.sidorin]

Some comments inline.

lib/AST/ASTImporter.cpp

4233

DC is imported first so it is possible to have a failed import and non-null DC. The assertion here seems to be incorrect.

4276

I think this should be placed into a separate function: there are many cases that can require attribute import so this code can be reused.

5815

This will assert if isImplicit() method returns true. In such case, Base should be nullptr.

5820

Are any of source parts can be null? Do we need to check them all?

5824

I have placed a function template for import of TemplateArgumentListInfo into D38845.

5848

Should we fail if nullptr was returned?

unittests/AST/ASTImporterTest.cpp

502

Could you please format the code with indents?

[szepet]

Updates based on comments.

[szepet]

lib/AST/ASTImporter.cpp

4276

Actually, I just realized it is part of the ´Importer.Imported´ function call as well, so this is unnecessary.

[szepet]

Left some unformatted lines in the previous version. Fixed.

[szepet]

lib/AST/ASTImporter.cpp

5848

Since E->getFirstQualifierFoundInScope() can be null, it does not necessarily mean that it should fail. However, I checked it in a more structured way above if its OK.

[a.sidorin]

Hi Peter. All is almost OK but I have some doubts about one of checking conditions.

lib/AST/ASTImporter.cpp

5831

Did you mean _!_E->getMember.isEmpty()? If so, you can rewrite the code as if (E->getMember() && !Name).

[szepet]

Updating the usage of ImportTemplateArgumentListInfo.

[a.sidorin]

lib/AST/ASTImporter.cpp

5835

I still have questions about this line. I strongly believe that we should fail if the source DeclarationName is non-empty and the result is empty. This line fails if they are both empty which seems to be a correct case for me.

[szepet]

Yepp, I am not exactly sure how I managed to do it, but I've missed this comment last time. Fixed it.

[a.sidorin]

lib/AST/ASTImporter.cpp

5835

Peter, this condition still looks inverted to me. Am I missing something?

[szepet]

Nope, that was me. If this (and the dependencies) are looking good, I can commit them as well (added the dependencies ^^).

[a.sidorin]

LGTM, thank you!

[xazax.hun]

It was reverted due to tests failures on windows build bots.

[a.sidorin]

Peter, could you please fix the tests to correspond new format? I've made them pass with some changes (in inline comment).

cfe/trunk/unittests/AST/ASTImporterTest.cpp

519 ↗

(On Diff #127276)

1. For these two tests, C should be struct because t is a private member and is inaccessible from declToImport().
2. An additional line void foo() { declToImport<int>(); } should resolve the issue for both tests.

[a.sidorin]

Sorry for the noise, I have fixed the patch and re-committed it, but forgot to remove "Request changes" status.

lib/AST/ASTImporter.cpp

public:
     TemplateParameterList *ImportTemplateParameterList(
                                                  TemplateParameterList *Params);
     TemplateArgument ImportTemplateArgument(const TemplateArgument &From);
     Optional<TemplateArgumentLoc> ImportTemplateArgumentLoc(
         const TemplateArgumentLoc &TALoc);
     bool ImportTemplateArguments(const TemplateArgument *FromArgs,
                                  unsigned NumFromArgs,
                                SmallVectorImpl<TemplateArgument> &ToArgs);
+    template<typename InContainerTy>
+    bool ImportTemplateArgumentListInfo(const InContainerTy &Container,
+                                        TemplateArgumentListInfo &ToTAInfo);
     bool IsStructuralMatch(RecordDecl *FromRecord, RecordDecl *ToRecord,
                            bool Complain = true);
     bool IsStructuralMatch(VarDecl *FromVar, VarDecl *ToVar,
                            bool Complain = true);
     bool IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToRecord);
     bool IsStructuralMatch(EnumConstantDecl *FromEC, EnumConstantDecl *ToEC);
+    bool IsStructuralMatch(FunctionTemplateDecl *From,
+                           FunctionTemplateDecl *To);
     bool IsStructuralMatch(ClassTemplateDecl *From, ClassTemplateDecl *To);
     bool IsStructuralMatch(VarTemplateDecl *From, VarTemplateDecl *To);
     Decl *VisitDecl(Decl *D);
     Decl *VisitEmptyDecl(EmptyDecl *D);
     Decl *VisitAccessSpecDecl(AccessSpecDecl *D);
     Decl *VisitStaticAssertDecl(StaticAssertDecl *D);
     Decl *VisitTranslationUnitDecl(TranslationUnitDecl *D);
     Decl *VisitNamespaceDecl(NamespaceDecl *D);
public:
     Decl *VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D);
     Decl *VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D);
     Decl *VisitTemplateTemplateParmDecl(TemplateTemplateParmDecl *D);
     Decl *VisitClassTemplateDecl(ClassTemplateDecl *D);
     Decl *VisitClassTemplateSpecializationDecl(
                                             ClassTemplateSpecializationDecl *D);
     Decl *VisitVarTemplateDecl(VarTemplateDecl *D);
     Decl *VisitVarTemplateSpecializationDecl(VarTemplateSpecializationDecl *D);
+    Decl *VisitFunctionTemplateDecl(FunctionTemplateDecl *D);
 
     // Importing statements
     DeclGroupRef ImportDeclGroup(DeclGroupRef DG);
 
     Stmt *VisitStmt(Stmt *S);
     Stmt *VisitGCCAsmStmt(GCCAsmStmt *S);
     Stmt *VisitDeclStmt(DeclStmt *S);
     Stmt *VisitNullStmt(NullStmt *S);
public:
     Expr *VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *E);
     Expr *VisitCXXTemporaryObjectExpr(CXXTemporaryObjectExpr *CE);
     Expr *VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *E);
     Expr *VisitPackExpansionExpr(PackExpansionExpr *E);
     Expr *VisitCXXNewExpr(CXXNewExpr *CE);
     Expr *VisitCXXDeleteExpr(CXXDeleteExpr *E);
     Expr *VisitCXXConstructExpr(CXXConstructExpr *E);
     Expr *VisitCXXMemberCallExpr(CXXMemberCallExpr *E);
+    Expr *VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E);
     Expr *VisitExprWithCleanups(ExprWithCleanups *EWC);
     Expr *VisitCXXThisExpr(CXXThisExpr *E);
     Expr *VisitCXXBoolLiteralExpr(CXXBoolLiteralExpr *E);
     Expr *VisitMemberExpr(MemberExpr *E);
     Expr *VisitCallExpr(CallExpr *E);
     Expr *VisitInitListExpr(InitListExpr *E);
     Expr *VisitArrayInitLoopExpr(ArrayInitLoopExpr *E);
     Expr *VisitArrayInitIndexExpr(ArrayInitIndexExpr *E);
if (To.isNull() && !FromArgs[I].isNull())
       return true;
     
     ToArgs.push_back(To);
   }
   
   return false;
 }
 
+template<typename InContainerTy>
+bool ASTNodeImporter::ImportTemplateArgumentListInfo(const InContainerTy &Container,
+                                    TemplateArgumentListInfo &ToTAInfo) {
+  for (const auto &FromLoc : Container) {
+    if (auto ToLoc = ImportTemplateArgumentLoc(FromLoc))
+      ToTAInfo.addArgument(*ToLoc);
+    else
+      return true;
+  }
+  return false;
+}
+
 bool ASTNodeImporter::IsStructuralMatch(RecordDecl *FromRecord, 
                                         RecordDecl *ToRecord, bool Complain) {
   // Eliminate a potential failure point where we attempt to re-import
   // something we're trying to import while completing ToRecord.
   Decl *ToOrigin = Importer.GetOriginalDecl(ToRecord);
   if (ToOrigin) {
     RecordDecl *ToOriginRecord = dyn_cast<RecordDecl>(ToOrigin);
     if (ToOriginRecord)

 
 bool ASTNodeImporter::IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToEnum) {
   StructuralEquivalenceContext Ctx(Importer.getFromContext(),
                                    Importer.getToContext(),
                                    Importer.getNonEquivalentDecls());
   return Ctx.IsStructurallyEquivalent(FromEnum, ToEnum);
 }
 
+bool ASTNodeImporter::IsStructuralMatch(FunctionTemplateDecl *From,
+                                        FunctionTemplateDecl *To) {
+  StructuralEquivalenceContext Ctx(
+      Importer.getFromContext(), Importer.getToContext(),
+      Importer.getNonEquivalentDecls(), false, false);
+  return Ctx.IsStructurallyEquivalent(From, To);
+}
+
 bool ASTNodeImporter::IsStructuralMatch(EnumConstantDecl *FromEC,
                                         EnumConstantDecl *ToEC)
 {
   const llvm::APSInt &FromVal = FromEC->getInitVal();
   const llvm::APSInt &ToVal = ToEC->getInitVal();
 
   return FromVal.isSigned() == ToVal.isSigned() &&
          FromVal.getBitWidth() == ToVal.getBitWidth() &&
Decl *ASTNodeImporter::VisitVarTemplateSpecializationDecl(
   Importer.Imported(D, D2);
 
   if (D->isThisDeclarationADefinition() && ImportDefinition(D, D2))
     return nullptr;
 
   return D2;
 }
 
+Decl *ASTNodeImporter::VisitFunctionTemplateDecl(FunctionTemplateDecl *D) {
+  DeclContext *DC, *LexicalDC;
+  DeclarationName Name;
+  SourceLocation Loc;
+  NamedDecl *ToD;
+
+  if (ImportDeclParts(D, DC, LexicalDC, Name, ToD, Loc))
+    return nullptr;
+
+  if (ToD)
+    return ToD;
+
+  // Try to find a function in our own ("to") context with the same name, same
+  // type, and in the same context as the function we're importing.
+  if (!LexicalDC->isFunctionOrMethod()) {
+    unsigned IDNS = Decl::IDNS_Ordinary;
+    SmallVector<NamedDecl *, 2> FoundDecls;
+    DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+    for (unsigned I = 0, N = FoundDecls.size(); I != N; ++I) {
+      if (!FoundDecls[I]->isInIdentifierNamespace(IDNS))
+        continue;
+
+      if (FunctionTemplateDecl *FoundFunction =
+              dyn_cast<FunctionTemplateDecl>(FoundDecls[I])) {
+        if (FoundFunction->hasExternalFormalLinkage() &&
+            D->hasExternalFormalLinkage()) {
+          if (IsStructuralMatch(D, FoundFunction)) {
+            Importer.Imported(D, FoundFunction);
+            // FIXME: Actually try to merge the body and other attributes.
+            return FoundFunction;
+          }
+        }
+      }
+    }
+  }
+
+  TemplateParameterList *Params =
+      ImportTemplateParameterList(D->getTemplateParameters());
+  if (!Params)
+    return nullptr;
+
+  FunctionDecl *TemplatedFD =
+      cast_or_null<FunctionDecl>(Importer.Import(D->getTemplatedDecl()));
+  if (!TemplatedFD)
+    return nullptr;
+
+  FunctionTemplateDecl *ToFunc = FunctionTemplateDecl::Create(
+      Importer.getToContext(), DC, Loc, Name, Params, TemplatedFD);
+
+  TemplatedFD->setDescribedFunctionTemplate(ToFunc);
+  ToFunc->setAccess(D->getAccess());
+  ToFunc->setLexicalDeclContext(LexicalDC);
+  Importer.Imported(D, ToFunc);
+
+  LexicalDC->addDeclInternal(ToFunc);
+  return ToFunc;
+}
+
 //----------------------------------------------------------------------------
 // Import Statements
 //----------------------------------------------------------------------------
 
 DeclGroupRef ASTNodeImporter::ImportDeclGroup(DeclGroupRef DG) {
   if (DG.isNull())
     return DeclGroupRef::Create(Importer.getToContext(), nullptr, 0);
   size_t NumDecls = DG.end() - DG.begin();
return MemberExpr::Create(Importer.getToContext(), ToBase,
                             Importer.Import(E->getOperatorLoc()),
                             Importer.Import(E->getQualifierLoc()),
                             Importer.Import(E->getTemplateKeywordLoc()),
                             ToMember, ToFoundDecl, ToMemberNameInfo,
                             nullptr, T, E->getValueKind(),
                             E->getObjectKind());
 }
 
+Expr *ASTNodeImporter::VisitCXXDependentScopeMemberExpr(
+    CXXDependentScopeMemberExpr *E) {
+  Expr* Base = nullptr;
+  if (!E->isImplicitAccess()) {
+    Base = Importer.Import(E->getBase());
+    if (!Base)
+      return nullptr;
+  }
+
+  QualType BaseType = Importer.Import(E->getBaseType());
+  if (BaseType.isNull())
+    return nullptr;
+
+  TemplateArgumentListInfo ToTAInfo(Importer.Import(E->getLAngleLoc()),
+                                    Importer.Import(E->getRAngleLoc()));
+  if (ImportTemplateArgumentListInfo(E->template_arguments(), ToTAInfo))
+   return nullptr;
+
+  DeclarationName Name = Importer.Import(E->getMember());
+  if (E->getMember().isEmpty() && Name.isEmpty())
+    return nullptr;
+
+  DeclarationNameInfo MemberNameInfo(Name, Importer.Import(E->getMemberLoc()));
+  // Import additional name location/type info.
+  ImportDeclarationNameLoc(E->getMemberNameInfo(), MemberNameInfo);
+  auto ToFQ = Importer.Import(E->getFirstQualifierFoundInScope());
+  if (ToFQ)
+    return nullptr;
+
+  return CXXDependentScopeMemberExpr::Create(
+      Importer.getToContext(), Base, BaseType, E->isArrow(),
+      Importer.Import(E->getOperatorLoc()),
+      Importer.Import(E->getQualifierLoc()),
+      Importer.Import(E->getTemplateKeywordLoc()),
+      cast<NamedDecl>(ToFQ), MemberNameInfo, &ToTAInfo);
+}
+
 Expr *ASTNodeImporter::VisitCallExpr(CallExpr *E) {
   QualType T = Importer.Import(E->getType());
   if (T.isNull())
     return nullptr;
 
   Expr *ToCallee = Importer.Import(E->getCallee());
   if (!ToCallee && E->getCallee())
     return nullptr;

unittests/AST/ASTImporterTest.cpp

EXPECT_TRUE(testImport("template <int K>"
                          "int declToImport() { return dummy2<3>::i; }",
                          Lang_CXX11, "", Lang_CXX11, Verifier,
                          functionDecl(
                            hasBody(
                              compoundStmt(
                                has(
                                  returnStmt(
                                    has(
                                      implicitCastExpr(
                                        has(
                                          declRefExpr()))))))))));
 }
 
 
 TEST(ImportType, ImportPackExpansion) {
   MatchVerifier<Decl> Verifier;
   EXPECT_TRUE(testImport("template <typename... Args>"
EXPECT_TRUE(testImport("template <typename... Args>"
                                has(
                                  returnStmt(
                                    has(
                                      implicitCastExpr(
                                        has(
                                          declRefExpr()))))))))));
 }
 
+TEST(ImportDecl, ImportFunctionTemplateDecl) {
+  MatchVerifier<Decl> Verifier;
+  EXPECT_TRUE(testImport("template <typename T> void declToImport() { };",
+                         Lang_CXX, "", Lang_CXX, Verifier,
+                         functionTemplateDecl()));
+}
+
+const internal::VariadicDynCastAllOfMatcher<Expr, CXXDependentScopeMemberExpr>
+    cxxDependentScopeMemberExpr;
+
+TEST(ImportExpr, ImportCXXDependentScopeMemberExpr) {
+  MatchVerifier<Decl> Verifier;
+  EXPECT_TRUE(testImport("template <typename T> class C { T t; };"
+                         "template <typename T> void declToImport() {"
+                         "  C<T> d;"
+                         "  d.t;"
+                         "}",
+                         Lang_CXX, "", Lang_CXX, Verifier,
+                         functionTemplateDecl(has(functionDecl(has(compoundStmt(
+                             has(cxxDependentScopeMemberExpr()))))))));
+  EXPECT_TRUE(testImport("template <typename T> class C { T t; };"
+                         "template <typename T> void declToImport() {"
+                         "  C<T> d;"
+                         "  (&d)->t;"
+                         "}",
+                         Lang_CXX, "", Lang_CXX, Verifier,
+                         functionTemplateDecl(has(functionDecl(has(compoundStmt(
+                             has(cxxDependentScopeMemberExpr()))))))));
+}
 
 } // end namespace ast_matchers
 } // end namespace clang