[ASTImporter] Support import of CXXDeductionGuideDecl

[martong]

Summary

CXXDeductionGuideDecl is a FunctionDecl, but its constructor should be called
appropriately, at least to set the kind variable properly.

[teemperor]

You think it's worth it to have a test for a user-specified deduction guide? I think this is missing an 'explicit' deduction guide test, so maybe we could test both things in a single additional test case.

Also, IIUC we ignore the IsCopyDeductionCandidate bit when importing and that seems like a potential bug to me (the value is apparently used for some overload resolution logic)?

Beside those two things this looks good to me.

clang/lib/AST/ASTImporter.cpp

3332

typo "funcitons"

[martong]

• Add test for user-defined ctad
• Handle IsCopyDeductionCandidate
• Fix typo

[martong]

Thanks for the review! Your comment made me think about whether we handle the deduced template class properly (getDeducedTemplate). Then I realized we do import that when we import the DeclarationName. Anyway I added a check for that in the test.

[teemperor]

I think the test could be made a bit stricter (see inline comment), but otherwise this LGTM. Thanks!

clang/unittests/AST/ASTImporterTest.cpp

5973

Not sure if that's actually testing what it should. FromD is not the copy deduction candidate, so this is just comparing that both Decls have the default value false?

If we pick the copy deduction candidate in this test then we could test that we actually copy the value over and not just have the default 'false' flag for isCopyDeductionCandidate (something like cxxDeductionGuideDecl(hasParameter(0, hasType(asString("A<T>")))) should find the copy deduction candidate).

Then we could also simplify this to EXPECT_TRUE(ToD->isCopyDeductionCandidate());.

[martong]

• Update test to match the copy deduction guide

clang/unittests/AST/ASTImporterTest.cpp

5973

Yeah, good point, updated so.

[rnk]

clang/lib/AST/ASTImporter.cpp

3336

GCC 5.3 complained about this, so I rewrote it a different way without generic lambdas in rG43b5b485a203f190ee4d5d3cab19c44ca865d316.

Error spew:

FAILED: tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/ASTImporter.cpp.o 
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc530trusty/bin/g++  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/lib/AST -I/b/s/w/ir/cache/builder/src/third_party/llvm/clang/lib/AST -I/b/s/w/ir/cache/builder/src/third_party/llvm/clang/include -Itools/clang/include -Iinclude -I/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include -DLLVM_FORCE_HEAD_REVISION -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O3     -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -UNDEBUG -std=c++14 -MD -MT tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/ASTImporter.cpp.o -MF tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/ASTImporter.cpp.o.d -o tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/ASTImporter.cpp.o -c /b/s/w/ir/cache/builder/src/third_party/llvm/clang/lib/AST/ASTImporter.cpp
/b/s/w/ir/cache/builder/src/third_party/llvm/clang/lib/AST/ASTImporter.cpp: In instantiation of ‘clang::ASTNodeImporter::VisitFunctionDecl(clang::FunctionDecl*)::<lambda(auto:1*)> [with auto:1 = clang::CXXConversionDecl; clang::ExpectedExpr = llvm::Expected<clang::Expr*>]’:
/b/s/w/ir/cache/builder/src/third_party/llvm/clang/lib/AST/ASTImporter.cpp:3384:66:   required from here
/b/s/w/ir/cache/builder/src/third_party/llvm/clang/lib/AST/ASTImporter.cpp:3336:20: error: cannot call member function ‘T clang::ASTNodeImporter::importChecked(llvm::Error&, const T&) [with T = clang::Expr*]’ without object
       ExplicitExpr = importChecked(Err, Fun->getExplicitSpecifier().getExpr());
                    ^
/b/s/w/ir/cache/builder/src/third_party/llvm/clang/lib/AST/ASTImporter.cpp: In instantiation of ‘clang::ASTNodeImporter::VisitFunctionDecl(clang::FunctionDecl*)::<lambda(auto:1*)> [with auto:1 = clang::CXXDeductionGuideDecl; clang::ExpectedExpr = llvm::Expected<clang::Expr*>]’:
/b/s/w/ir/cache/builder/src/third_party/llvm/clang/lib/AST/ASTImporter.cpp:3402:57:   required from here
/b/s/w/ir/cache/builder/src/third_party/llvm/clang/lib/AST/ASTImporter.cpp:3336:20: error: cannot call member function ‘T clang::ASTNodeImporter::importChecked(llvm::Error&, const T&) [with T = clang::Expr*]’ without object
[3456/5141] Building CXX object tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/InheritViz.cpp.o

[martong]

clang/lib/AST/ASTImporter.cpp

3336

Thanks, post-commit looks good!

clang/lib/AST/ASTImporter.cpp

public:
     ExpectedDecl VisitEnumDecl(EnumDecl *D);
     ExpectedDecl VisitRecordDecl(RecordDecl *D);
     ExpectedDecl VisitEnumConstantDecl(EnumConstantDecl *D);
     ExpectedDecl VisitFunctionDecl(FunctionDecl *D);
     ExpectedDecl VisitCXXMethodDecl(CXXMethodDecl *D);
     ExpectedDecl VisitCXXConstructorDecl(CXXConstructorDecl *D);
     ExpectedDecl VisitCXXDestructorDecl(CXXDestructorDecl *D);
     ExpectedDecl VisitCXXConversionDecl(CXXConversionDecl *D);
+    ExpectedDecl VisitCXXDeductionGuideDecl(CXXDeductionGuideDecl *D);
     ExpectedDecl VisitFieldDecl(FieldDecl *D);
     ExpectedDecl VisitIndirectFieldDecl(IndirectFieldDecl *D);
     ExpectedDecl VisitFriendDecl(FriendDecl *D);
     ExpectedDecl VisitObjCIvarDecl(ObjCIvarDecl *D);
     ExpectedDecl VisitVarDecl(VarDecl *D);
     ExpectedDecl VisitImplicitParamDecl(ImplicitParamDecl *D);
     ExpectedDecl VisitParmVarDecl(ParmVarDecl *D);
     ExpectedDecl VisitObjCMethodDecl(ObjCMethodDecl *D);
ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
   SmallVector<ParmVarDecl *, 8> Parameters;
   for (auto P : D->parameters()) {
     if (Expected<ParmVarDecl *> ToPOrErr = import(P))
       Parameters.push_back(*ToPOrErr);
     else
       return ToPOrErr.takeError();
   }
 
+  // Common code to import an explicit specifier of different kind of functions.
+  auto ImportExplicitExpr = [this, &Err](auto *Fun) -> ExpectedExpr {
+    Expr *ExplicitExpr = nullptr;
+    if (Fun->getExplicitSpecifier().getExpr()) {
+      ExplicitExpr = importChecked(Err, Fun->getExplicitSpecifier().getExpr());
+      if (Err)
+        return std::move(Err);
+    }
+    return ExplicitExpr;
+  };
+
   // Create the imported function.
   FunctionDecl *ToFunction = nullptr;
   if (auto *FromConstructor = dyn_cast<CXXConstructorDecl>(D)) {
     Expr *ExplicitExpr = nullptr;
     if (FromConstructor->getExplicitSpecifier().getExpr()) {
       auto Imp = import(FromConstructor->getExplicitSpecifier().getExpr());
       if (!Imp)
         return Imp.takeError();
if (GetImportedOrCreateDecl<CXXDestructorDecl>(
         D->isImplicit(), D->getConstexprKind(), TrailingRequiresClause))
       return ToFunction;
 
     CXXDestructorDecl *ToDtor = cast<CXXDestructorDecl>(ToFunction);
 
     ToDtor->setOperatorDelete(ToOperatorDelete, ToThisArg);
   } else if (CXXConversionDecl *FromConversion =
                  dyn_cast<CXXConversionDecl>(D)) {
-    Expr *ExplicitExpr = nullptr;
-    if (FromConversion->getExplicitSpecifier().getExpr()) {
-      auto Imp = import(FromConversion->getExplicitSpecifier().getExpr());
-      if (!Imp)
-        return Imp.takeError();
-      ExplicitExpr = *Imp;
-    }
+    ExpectedExpr ExplicitExpr = ImportExplicitExpr(FromConversion);
+    if (!ExplicitExpr)
+      return ExplicitExpr.takeError();
     if (GetImportedOrCreateDecl<CXXConversionDecl>(
             ToFunction, D, Importer.getToContext(), cast<CXXRecordDecl>(DC),
             ToInnerLocStart, NameInfo, T, TInfo, D->isInlineSpecified(),
-            ExplicitSpecifier(ExplicitExpr,
+            ExplicitSpecifier(*ExplicitExpr,
                               FromConversion->getExplicitSpecifier().getKind()),
             D->getConstexprKind(), SourceLocation(), TrailingRequiresClause))
       return ToFunction;
   } else if (auto *Method = dyn_cast<CXXMethodDecl>(D)) {
     if (GetImportedOrCreateDecl<CXXMethodDecl>(
             ToFunction, D, Importer.getToContext(), cast<CXXRecordDecl>(DC),
             ToInnerLocStart, NameInfo, T, TInfo, Method->getStorageClass(),
             Method->isInlineSpecified(), D->getConstexprKind(),
             SourceLocation(), TrailingRequiresClause))
       return ToFunction;
+  } else if (auto *Guide = dyn_cast<CXXDeductionGuideDecl>(D)) {
+    ExpectedExpr ExplicitExpr = ImportExplicitExpr(Guide);
+    if (!ExplicitExpr)
+      return ExplicitExpr.takeError();
+    if (GetImportedOrCreateDecl<CXXDeductionGuideDecl>(
+            ToFunction, D, Importer.getToContext(), DC, ToInnerLocStart,
+            ExplicitSpecifier(*ExplicitExpr,
+                              Guide->getExplicitSpecifier().getKind()),
+            NameInfo, T, TInfo, ToEndLoc))
+      return ToFunction;
+    cast<CXXDeductionGuideDecl>(ToFunction)
+        ->setIsCopyDeductionCandidate(Guide->isCopyDeductionCandidate());
   } else {
     if (GetImportedOrCreateDecl(
             ToFunction, D, Importer.getToContext(), DC, ToInnerLocStart,
             NameInfo, T, TInfo, D->getStorageClass(), D->isInlineSpecified(),
             D->hasWrittenPrototype(), D->getConstexprKind(),
             TrailingRequiresClause))
       return ToFunction;
   }

 ExpectedDecl ASTNodeImporter::VisitCXXDestructorDecl(CXXDestructorDecl *D) {
   return VisitCXXMethodDecl(D);
 }
 
 ExpectedDecl ASTNodeImporter::VisitCXXConversionDecl(CXXConversionDecl *D) {
   return VisitCXXMethodDecl(D);
 }
 
+ExpectedDecl
+ASTNodeImporter::VisitCXXDeductionGuideDecl(CXXDeductionGuideDecl *D) {
+  return VisitFunctionDecl(D);
+}
+
 ExpectedDecl ASTNodeImporter::VisitFieldDecl(FieldDecl *D) {
   // Import the major distinguishing characteristics of a variable.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
   SourceLocation Loc;
   NamedDecl *ToD;
   if (Error Err = ImportDeclParts(D, DC, LexicalDC, Name, ToD, Loc))
     return std::move(Err);

clang/unittests/AST/ASTImporterTest.cpp

TEST_P(ImportWithExternalSource, CompleteRecordBeforeImporting) {
   EXPECT_FALSE((bool)Err);
   consumeError(std::move(Err));
 
   // Make sure the class was completed once.
   EXPECT_EQ(1U, CompletedTags.size());
   EXPECT_EQ(Record, CompletedTags.front());
 }
 
+TEST_P(ImportFunctions, CTADImplicit) {
+  Decl *FromTU = getTuDecl(
+      R"(
+      template <typename T> struct A {
+        A(T);
+      };
+      A a{(int)0};
+      )",
+      Lang_CXX17, "input.cc");
+  auto *FromD = FirstDeclMatcher<CXXDeductionGuideDecl>().match(
+      FromTU,
+      cxxDeductionGuideDecl(hasParameter(0, hasType(asString("A<T>")))));
+  auto *ToD = Import(FromD, Lang_CXX17);
+  ASSERT_TRUE(ToD);
+  EXPECT_TRUE(ToD->isCopyDeductionCandidate());
+  // Check that the deduced class template is also imported.
+  EXPECT_TRUE(findFromTU(FromD)->Importer->GetAlreadyImportedOrNull(
+      FromD->getDeducedTemplate()));
+}
+
+TEST_P(ImportFunctions, CTADUserDefinedExplicit) {
+  Decl *FromTU = getTuDecl(
+      R"(
+      template <typename T> struct A {
+        A(T);
+      };
+      template <typename T> explicit A(T) -> A<float>;
+      A a{(int)0}; // calls A<float>::A(float)
+      )",
+      Lang_CXX17, "input.cc");
+  auto *FromD = FirstDeclMatcher<CXXDeductionGuideDecl>().match(
+      FromTU, cxxDeductionGuideDecl(unless(isImplicit())));
+  // Not-implicit: i.e. not compiler-generated, user defined.
+  ASSERT_FALSE(FromD->isImplicit());
+  ASSERT_TRUE(FromD->isExplicit()); // Has the explicit keyword.
+  auto *ToD = Import(FromD, Lang_CXX17);
+  ASSERT_TRUE(ToD);
+  EXPECT_FALSE(FromD->isImplicit());
+  EXPECT_TRUE(ToD->isExplicit());
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
                         DefaultTestValuesForRunOptions, );
 
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportPath,
                         ::testing::Values(std::vector<std::string>()), );
 
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportExpr,
                         DefaultTestValuesForRunOptions, );