[ASTImporter] Import implicit methods of existing class.

[balazske]

Summary

When an already existing class is encountered during import,
check if it has implicit methods that are missing in the existing one,
and import these.
The to-be-imported code may use the same class in different way than the
existing (before the import) code. This may result in that there are
implicit methods that are not generated for the existing code.

[a_sidorin]

LGTM. Just some stylish nits.
To resolve this issue, I used Sema::DeclareImplicit... methods. But I like this approach much more because it doesn't allows to forget different kinds of implicit methods and doesn't require passing Sema into ASTImporter.

lib/AST/ASTImporter.cpp

2203

... because implicit methods are created only if they are used.

unittests/AST/ASTImporterTest.cpp

2302

/*ExpectedCount=*/. Same below.

2305

clang-format places a space before the template parameter list.

[balazske]

• Small style and comment changes.

lib/AST/ASTImporter.cpp

public:
     // Importing declarations
     bool ImportDeclParts(NamedDecl *D, DeclContext *&DC, 
                          DeclContext *&LexicalDC, DeclarationName &Name, 
                          NamedDecl *&ToD, SourceLocation &Loc);
     void ImportDefinitionIfNeeded(Decl *FromD, Decl *ToD = nullptr);
     void ImportDeclarationNameLoc(const DeclarationNameInfo &From,
                                   DeclarationNameInfo& To);
     void ImportDeclContext(DeclContext *FromDC, bool ForceImport = false);
+    void ImportImplicitMethods(const CXXRecordDecl *From, CXXRecordDecl *To);
 
     bool ImportCastPath(CastExpr *E, CXXCastPath &Path);
 
     using Designator = DesignatedInitExpr::Designator;
 
     Designator ImportDesignator(const Designator &D);
 
     Optional<LambdaCapture> ImportLambdaCapture(const LambdaCapture &From);
if (Importer.isMinimalImport() && !ForceImport) {
     Importer.ImportContext(FromDC);
     return;
   }
   
   for (auto *From : FromDC->decls())
     Importer.Import(From);
 }
 
+void ASTNodeImporter::ImportImplicitMethods(
+    const CXXRecordDecl *From, CXXRecordDecl *To) {
+  assert(From->isCompleteDefinition() && To->getDefinition() == To &&
+      "Import implicit methods to or from non-definition");
+  
+  for (CXXMethodDecl *FromM : From->methods())
+    if (FromM->isImplicit())
+      Importer.Import(FromM);
+}
+
 static void setTypedefNameForAnonDecl(TagDecl *From, TagDecl *To,
                                ASTImporter &Importer) {
   if (TypedefNameDecl *FromTypedef = From->getTypedefNameForAnonDecl()) {
     auto *ToTypedef =
       cast_or_null<TypedefNameDecl>(Importer.Import(FromTypedef));
     assert (ToTypedef && "Failed to import typedef of an anonymous structure");
 
     To->setTypedefNameForAnonDecl(ToTypedef);
for (auto *FoundDecl : FoundDecls) {
           if ((SearchName && !D->isCompleteDefinition())
               || (D->isCompleteDefinition() &&
                   D->isAnonymousStructOrUnion()
                     == FoundDef->isAnonymousStructOrUnion() &&
                   IsStructuralMatch(D, FoundDef))) {
             // The record types structurally match, or the "from" translation
             // unit only had a forward declaration anyway; call it the same
             // function.
-            // FIXME: For C++, we should also merge methods here.
-            return Importer.MapImported(D, FoundDef);
+            // FIXME: Structural equivalence check should check for same
+            // user-defined methods.
+            Importer.MapImported(D, FoundDef);
+            if (const auto *DCXX = dyn_cast<CXXRecordDecl>(D)) {
+              auto *FoundCXX = dyn_cast<CXXRecordDecl>(FoundDef);
+              assert(FoundCXX && "Record type mismatch");
+
+              if (D->isCompleteDefinition() && !Importer.isMinimalImport())
+                // FoundDef may not have every implicit method that D has.
+                ImportImplicitMethods(DCXX, FoundCXX);
+            }
+            return FoundDef;
           }
         } else if (!D->isCompleteDefinition()) {
           // We have a forward declaration of this type, so adopt that forward
           // declaration rather than building a new one.
             
           // If one or both can be completed from external storage then try one
           // last time to complete and compare them before doing this.
             

unittests/AST/ASTImporterTest.cpp

testImport("struct S { template <typename T> void mem(); };"
              "  s.mem<U>();"
              "}"
              "void instantiate() { declToImport<int>(); }",
              Lang_CXX11, "", Lang_CXX11, Verifier,
              functionTemplateDecl(has(functionDecl(has(
                  compoundStmt(has(callExpr(has(unresolvedMemberExpr())))))))));
 }
 
+class ImportImplicitMethods : public ASTImporterTestBase {
+public:
+  static constexpr auto DefaultCode = R"(
+      struct A { int x; };
+      void f() {
+        A a;
+        A a1(a);
+        A a2(A{});
+        a = a1;
+        a = A{};
+        a.~A();
+      })";
+
+  template<typename MatcherType>
+  void testImportOf(
+      const MatcherType &MethodMatcher, const char *Code = DefaultCode) {
+    test(MethodMatcher, Code, 1u);
+  }
+
+  template<typename MatcherType>
+  void testNoImportOf(
+      const MatcherType &MethodMatcher, const char *Code = DefaultCode) {
+    test(MethodMatcher, Code, 0u);
+  }
+
+private:
+  template<typename MatcherType>
+  void test(const MatcherType &MethodMatcher,
+      const char *Code = DefaultCode, unsigned int ExpectedCount = 1u) {
+    auto ClassMatcher = cxxRecordDecl(unless(isImplicit()));
+
+    Decl *ToTU = getToTuDecl(Code, Lang_CXX11);
+    auto *ToClass = FirstDeclMatcher<CXXRecordDecl>().match(
+        ToTU, ClassMatcher);
+
+    ASSERT_EQ(DeclCounter<CXXMethodDecl>().match(ToClass, MethodMatcher), 1);
+
+    {
+      CXXMethodDecl *Method =
+          FirstDeclMatcher<CXXMethodDecl>().match(ToClass, MethodMatcher);
+      ToClass->removeDecl(Method);
+    }
+
+    ASSERT_EQ(DeclCounter<CXXMethodDecl>().match(ToClass, MethodMatcher), 0);
+
+    Decl *ImportedClass = nullptr;
+    {
+      Decl *FromTU = getTuDecl(Code, Lang_CXX11, "input1.cc");
+      auto *FromClass = FirstDeclMatcher<CXXRecordDecl>().match(
+          FromTU, ClassMatcher);
+      ImportedClass = Import(FromClass, Lang_CXX11);
+    }
+
+    EXPECT_EQ(ToClass, ImportedClass);
+    EXPECT_EQ(DeclCounter<CXXMethodDecl>().match(ToClass, MethodMatcher),
+        ExpectedCount);
+  }
+};
+
+TEST_P(ImportImplicitMethods, DefaultConstructor) {
+  testImportOf(cxxConstructorDecl(isDefaultConstructor()));
+}
+
+TEST_P(ImportImplicitMethods, CopyConstructor) {
+  testImportOf(cxxConstructorDecl(isCopyConstructor()));
+}
+
+TEST_P(ImportImplicitMethods, MoveConstructor) {
+  testImportOf(cxxConstructorDecl(isMoveConstructor()));
+}
+
+TEST_P(ImportImplicitMethods, Destructor) {
+  testImportOf(cxxDestructorDecl());
+}
+
+TEST_P(ImportImplicitMethods, CopyAssignment) {
+  testImportOf(cxxMethodDecl(isCopyAssignmentOperator()));
+}
+
+TEST_P(ImportImplicitMethods, MoveAssignment) {
+  testImportOf(cxxMethodDecl(isMoveAssignmentOperator()));
+}
+
+TEST_P(ImportImplicitMethods, DoNotImportUserProvided) {
+  auto Code = R"(
+      struct A { A() { int x; } };
+      )";
+  testNoImportOf(cxxConstructorDecl(isDefaultConstructor()), Code);
+}
+
+TEST_P(ImportImplicitMethods, DoNotImportDefault) {
+  auto Code = R"(
+      struct A { A() = default; };
+      )";
+  testNoImportOf(cxxConstructorDecl(isDefaultConstructor()), Code);
+}
+
+TEST_P(ImportImplicitMethods, DoNotImportDeleted) {
+  auto Code = R"(
+      struct A { A() = delete; };
+      )";
+  testNoImportOf(cxxConstructorDecl(isDefaultConstructor()), Code);
+}
+
+TEST_P(ImportImplicitMethods, DoNotImportOtherMethod) {
+  auto Code = R"(
+      struct A { void f() { } };
+      )";
+  testNoImportOf(cxxMethodDecl(hasName("f")), Code);
+}
+
 TEST_P(ASTImporterTestBase, ImportOfEquivalentRecord) {
   Decl *ToR1;
   {
     Decl *FromTU = getTuDecl(
         "struct A { };", Lang_CXX, "input0.cc");
     auto *FromR = FirstDeclMatcher<CXXRecordDecl>().match(
         FromTU, cxxRecordDecl(hasName("A")));