[modules] When merging class definitions, make the retained definition visible

if the merged definition is visible, and perform lookups into all merged copies
of the definition (not just for special members) so that we can complete the
redecl chains for members of the class.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@233420 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index 4d5141bf92159a0baaf7f12cd87cb3d98f4d5d80..73c44958611ce33d307090187351a02d30672381 100644 (file)
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -6730,20 +6730,14 @@ ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC,
   // individually, because finding an entity in one of them doesn't imply that
   // we can't find a different entity in another one.
   if (isa<CXXRecordDecl>(DC)) {
-    auto Kind = Name.getNameKind();
-    if (Kind == DeclarationName::CXXConstructorName ||
-        Kind == DeclarationName::CXXDestructorName ||
-        (Kind == DeclarationName::CXXOperatorName &&
-         Name.getCXXOverloadedOperator() == OO_Equal)) {
-      auto Merged = MergedLookups.find(DC);
-      if (Merged != MergedLookups.end()) {
-        for (unsigned I = 0; I != Merged->second.size(); ++I) {
-          const DeclContext *Context = Merged->second[I];
-          LookUpInContexts(Context);
-          // We might have just added some more merged lookups. If so, our
-          // iterator is now invalid, so grab a fresh one before continuing.
-          Merged = MergedLookups.find(DC);
-        }
+    auto Merged = MergedLookups.find(DC);
+    if (Merged != MergedLookups.end()) {
+      for (unsigned I = 0; I != Merged->second.size(); ++I) {
+        const DeclContext *Context = Merged->second[I];
+        LookUpInContexts(Context);
+        // We might have just added some more merged lookups. If so, our
+        // iterator is now invalid, so grab a fresh one before continuing.
+        Merged = MergedLookups.find(DC);
       }
     }
   }

diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index de2c625306929461f40fabd9e09b4744eda97b8a..5e911b41b7c43707b3ca4c8a446c2aff79fe56c5 100644 (file)
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -1385,6 +1385,29 @@ void ASTDeclReader::MergeDefinitionData(
          "merging class definition into non-definition");
   auto &DD = *D->DefinitionData.getNotUpdated();
 
+  // If the new definition has new special members, let the name lookup
+  // code know that it needs to look in the new definition too.
+  //
+  // FIXME: We only need to do this if the merged definition declares members
+  // that this definition did not declare, or if it defines members that this
+  // definition did not define.
+  if (DD.Definition != MergeDD.Definition) {
+    Reader.MergedLookups[DD.Definition].push_back(MergeDD.Definition);
+    DD.Definition->setHasExternalVisibleStorage();
+
+    if (DD.Definition->isHidden()) {
+      // If MergeDD is visible or becomes visible, make the definition visible.
+      if (!MergeDD.Definition->isHidden())
+        DD.Definition->Hidden = false;
+      else {
+        auto SubmoduleID = MergeDD.Definition->getOwningModuleID();
+        assert(SubmoduleID && "hidden definition in no module");
+        Reader.HiddenNamesMap[Reader.getSubmodule(SubmoduleID)]
+              .HiddenDecls.push_back(DD.Definition);
+      }
+    }
+  }
+
   auto PFDI = Reader.PendingFakeDefinitionData.find(&DD);
   if (PFDI != Reader.PendingFakeDefinitionData.end() &&
       PFDI->second == ASTReader::PendingFakeDefinitionKind::Fake) {
@@ -1401,17 +1424,6 @@ void ASTDeclReader::MergeDefinitionData(
     return;
   }
 
-  // If the new definition has new special members, let the name lookup
-  // code know that it needs to look in the new definition too.
-  //
-  // FIXME: We only need to do this if the merged definition declares members
-  // that this definition did not declare, or if it defines members that this
-  // definition did not define.
-  if (MergeDD.DeclaredSpecialMembers && DD.Definition != MergeDD.Definition) {
-    Reader.MergedLookups[DD.Definition].push_back(MergeDD.Definition);
-    DD.Definition->setHasExternalVisibleStorage();
-  }
-
   // FIXME: Move this out into a .def file?
   bool DetectedOdrViolation = false;
 #define OR_FIELD(Field) DD.Field |= MergeDD.Field;

diff --git a/test/Modules/Inputs/redecl-add-after-load-decls.h b/test/Modules/Inputs/redecl-add-after-load-decls.h
index fbe6b9387a1705e06ebfeb7d84a57dcf92318589..a4227971a876317af1737ce34ec272672788fda0 100644 (file)
--- a/test/Modules/Inputs/redecl-add-after-load-decls.h
+++ b/test/Modules/Inputs/redecl-add-after-load-decls.h
@@ -16,9 +16,9 @@ typedef C::A CB;
 constexpr int C_test(bool b) { return b ? C::variable : C::function(); }
 
 struct D {
-  struct A; // expected-note {{forward}}
+  struct A;
   static const int variable;
-  static constexpr int function(); // expected-note {{here}}
+  static constexpr int function();
 };
 typedef D::A DB;
-constexpr int D_test(bool b) { return b ? D::variable : D::function(); } // expected-note {{subexpression}} expected-note {{undefined}}
+constexpr int D_test(bool b) { return b ? D::variable : D::function(); }

diff --git a/test/Modules/Inputs/submodules-merge-defs/module.modulemap b/test/Modules/Inputs/submodules-merge-defs/module.modulemap
index 5c7ee8a89adf4d3c4b2439a8e1e32f6c57f290df..82abdb088f196860c15437e29ddea6b27b961ebf 100644 (file)
--- a/test/Modules/Inputs/submodules-merge-defs/module.modulemap
+++ b/test/Modules/Inputs/submodules-merge-defs/module.modulemap
@@ -9,3 +9,8 @@ module "redef" {
   // Do not re-export stuff.use
   use "stuff"
 }
+
+module "merged-defs" {
+  header "merged-defs.h"
+  use "stuff"
+}

diff --git a/test/Modules/cxx-templates.cpp b/test/Modules/cxx-templates.cpp
index 41b0f2cd92d57ce3a10f1f8dda31979ff067dc5a..00fc38b52209369e44b92ad1f86ad0df5a4ccf18 100644 (file)
--- a/test/Modules/cxx-templates.cpp
+++ b/test/Modules/cxx-templates.cpp
@@ -125,10 +125,7 @@ void g() {
 static_assert(Outer<int>::Inner<int>::f() == 1, "");
 static_assert(Outer<int>::Inner<int>::g() == 2, "");
 
-// FIXME: We're too lazy in merging class definitions to find the definition
-// of this function.
-static_assert(MergeTemplateDefinitions<int>::f() == 1, ""); // expected-error {{constant expression}} expected-note {{undefined}}
-// expected-note@cxx-templates-c.h:10 {{here}}
+static_assert(MergeTemplateDefinitions<int>::f() == 1, "");
 static_assert(MergeTemplateDefinitions<int>::g() == 2, "");
 
 RedeclaredAsFriend<int> raf1;

diff --git a/test/Modules/redecl-add-after-load.cpp b/test/Modules/redecl-add-after-load.cpp
index 53e54c84cc3a2dbcb097591492d287d466029e11..8ca70ad9580be969dcf7a8807e0e7c3e5d8191a9 100644 (file)
--- a/test/Modules/redecl-add-after-load.cpp
+++ b/test/Modules/redecl-add-after-load.cpp
@@ -2,8 +2,9 @@
 // RUN: %clang_cc1 -x objective-c++ -fmodules -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++11
 // RUN: %clang_cc1 -x objective-c++ -fmodules -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++11 -DIMPORT_DECLS
 
-#ifdef IMPORT_DECLS
 // expected-no-diagnostics
+
+#ifdef IMPORT_DECLS
 @import redecl_add_after_load_decls;
 #else
 typedef struct A B;
@@ -24,12 +25,12 @@ typedef C::A CB;
 constexpr int C_test(bool b) { return b ? C::variable : C::function(); }
 
 struct D {
-  struct A; // expected-note {{forward}}
+  struct A;
   static const int variable;
-  static constexpr int function(); // expected-note {{here}}
+  static constexpr int function();
 };
 typedef D::A DB;
-constexpr int D_test(bool b) { return b ? D::variable : D::function(); } // expected-note {{undefined}}
+constexpr int D_test(bool b) { return b ? D::variable : D::function(); }
 #endif
 
 @import redecl_add_after_load;
@@ -46,14 +47,6 @@ CB struct_struct_test;
 constexpr int struct_variable_test = C_test(true);
 constexpr int struct_function_test = C_test(false);
 
-// FIXME: We should accept this, but we're currently too lazy when merging class
-// definitions to determine that the definitions in redecl_add_after_load are
-// definitions of these entities.
 DB merged_struct_struct_test;
 constexpr int merged_struct_variable_test = D_test(true);
 constexpr int merged_struct_function_test = D_test(false);
-#ifndef IMPORT_DECLS
-// expected-error@-4 {{incomplete}}
-// @-4: definition of D::variable must be emitted, so it gets imported eagerly
-// expected-error@-4 {{constant}} expected-note@-4 {{in call to}}
-#endif

diff --git a/test/Modules/submodules-merge-defs.cpp b/test/Modules/submodules-merge-defs.cpp
index 86e50368a9ea153b6f32d8d4f792329f1b14d7ee..3bc52fee828f3bc487cf52aef58f032e6abf9c66 100644 (file)
--- a/test/Modules/submodules-merge-defs.cpp
+++ b/test/Modules/submodules-merge-defs.cpp
@@ -1,4 +1,5 @@
 // RUN: rm -rf %t
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules-cache-path=%t -fmodules -I %S/Inputs/submodules-merge-defs %s -verify -fno-modules-error-recovery -DTEXTUAL
 // RUN: %clang_cc1 -x c++ -std=c++11 -fmodules-cache-path=%t -fmodules -I %S/Inputs/submodules-merge-defs %s -verify -fno-modules-error-recovery
 
 // Trigger import of definitions, but don't make them visible.
@@ -27,7 +28,11 @@ D::X pre_dx; // expected-error +{{must be imported}}
 int pre_use_dx = use_dx(pre_dx);
 
 // Make definitions from second module visible.
+#ifdef TEXTUAL
 #include "import-and-redefine.h"
+#else
+#include "merged-defs.h"
+#endif
 
 A post_a;
 int post_use_a = use_a(post_a);