balazske created this revision.
Herald added subscribers: cfe-commits, martong.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: a.sidorin.

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.


Repository:
  rC Clang

https://reviews.llvm.org/D49245

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===================================================================
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2283,6 +2283,117 @@
                  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;
   {
Index: lib/AST/ASTImporter.cpp
===================================================================
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -228,6 +228,7 @@
     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);
 
@@ -1235,6 +1236,16 @@
     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()) {
@@ -2181,8 +2192,18 @@
             // 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to