Track definition merging on the canonical declaration even when local

submodule visibility is disabled.

Attempting to pick a specific declaration to make visible when the
module containing the merged declaration becomes visible is error-prone,
as we don't yet know which declaration we'll choose to be the definition
when we are informed of the merging.

This reinstates r342019, reverted in r342020. The regression previously
observed after this commit was fixed in r342096.

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

diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp
index 0ed52ffd351a603784fed6d91aba6f056160813f..b107e8bd2a3977cb253ac89635b6a6d0538277cb 100644 (file)
--- a/lib/AST/ASTContext.cpp
+++ b/lib/AST/ASTContext.cpp
@@ -932,10 +932,7 @@ void ASTContext::mergeDefinitionIntoModule(NamedDecl *ND, Module *M,
     if (auto *Listener = getASTMutationListener())
       Listener->RedefinedHiddenDefinition(ND, M);
 
-  if (getLangOpts().ModulesLocalVisibility)
-    MergedDefModules[cast<NamedDecl>(ND->getCanonicalDecl())].push_back(M);
-  else
-    ND->setVisibleDespiteOwningModule();
+  MergedDefModules[cast<NamedDecl>(ND->getCanonicalDecl())].push_back(M);
 }
 
 void ASTContext::deduplicateMergedDefinitonsFor(NamedDecl *ND) {

diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp
index 900ccb18c7ea16df75bb43c810ca0af852eecedc..819f9ef8a4872e7593980388b0d583f340c23c96 100644 (file)
--- a/lib/Sema/SemaType.cpp
+++ b/lib/Sema/SemaType.cpp
@@ -7623,8 +7623,15 @@ bool Sema::hasVisibleDefinition(NamedDecl *D, NamedDecl **Suggested,
 
     // A visible module might have a merged definition instead.
     if (D->isModulePrivate() ? hasMergedDefinitionInCurrentModule(D)
-                             : hasVisibleMergedDefinition(D))
+                             : hasVisibleMergedDefinition(D)) {
+      if (CodeSynthesisContexts.empty() &&
+          !getLangOpts().ModulesLocalVisibility) {
+        // Cache the fact that this definition is implicitly visible because
+        // there is a visible merged definition.
+        D->setVisibleDespiteOwningModule();
+      }
       return true;
+    }
 
     return false;
   };

diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index b60e5f14a6769783b472664e5536de53bf37b04a..ac3b7374018a9a576ba46604c5ca8e748a8eacd4 100644 (file)
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -3780,22 +3780,15 @@ void ASTReader::makeModuleVisible(Module *Mod,
 /// visible.
 void ASTReader::mergeDefinitionVisibility(NamedDecl *Def,
                                           NamedDecl *MergedDef) {
-  // FIXME: This doesn't correctly handle the case where MergedDef is visible
-  // in modules other than its owning module. We should instead give the
-  // ASTContext a list of merged definitions for Def.
   if (Def->isHidden()) {
     // If MergedDef is visible or becomes visible, make the definition visible.
     if (!MergedDef->isHidden())
       Def->setVisibleDespiteOwningModule();
-    else if (getContext().getLangOpts().ModulesLocalVisibility) {
+    else {
       getContext().mergeDefinitionIntoModule(
           Def, MergedDef->getImportedOwningModule(),
           /*NotifyListeners*/ false);
       PendingMergedDefinitionsToDeduplicate.insert(Def);
-    } else {
-      auto SubmoduleID = MergedDef->getOwningModuleID();
-      assert(SubmoduleID && "hidden definition in no module");
-      HiddenNamesMap[getSubmodule(SubmoduleID)].push_back(Def);
     }
   }
 }

diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index 00fad25eeac19fc92830a561f65b98facd59296d..829510747647d9676fd8f9c04357f89ca3c4fcba 100644 (file)
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -4419,22 +4419,9 @@ void ASTDeclReader::UpdateDecl(Decl *D,
     case UPD_DECL_EXPORTED: {
       unsigned SubmoduleID = readSubmoduleID();
       auto *Exported = cast<NamedDecl>(D);
-      if (auto *TD = dyn_cast<TagDecl>(Exported))
-        Exported = TD->getDefinition();
       Module *Owner = SubmoduleID ? Reader.getSubmodule(SubmoduleID) : nullptr;
-      if (Reader.getContext().getLangOpts().ModulesLocalVisibility) {
-        Reader.getContext().mergeDefinitionIntoModule(cast<NamedDecl>(Exported),
-                                                      Owner);
-        Reader.PendingMergedDefinitionsToDeduplicate.insert(
-            cast<NamedDecl>(Exported));
-      } else if (Owner && Owner->NameVisibility != Module::AllVisible) {
-        // If Owner is made visible at some later point, make this declaration
-        // visible too.
-        Reader.HiddenNamesMap[Owner].push_back(Exported);
-      } else {
-        // The declaration is now visible.
-        Exported->setVisibleDespiteOwningModule();
-      }
+      Reader.getContext().mergeDefinitionIntoModule(Exported, Owner);
+      Reader.PendingMergedDefinitionsToDeduplicate.insert(Exported);
       break;
     }
 

diff --git a/test/Modules/merge-template-pattern-visibility-3.cpp b/test/Modules/merge-template-pattern-visibility-3.cpp
new file mode 100644 (file)
index 0000000..9ac1b66
--- /dev/null
+++ b/test/Modules/merge-template-pattern-visibility-3.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -fmodules -emit-llvm-only %s -verify
+
+#pragma clang module build A
+module A {}
+#pragma clang module contents
+#pragma clang module begin A
+template<typename T> void f(const T&) { T::error; }
+#pragma clang module end
+#pragma clang module endbuild
+
+#pragma clang module build B
+module B {}
+#pragma clang module contents
+#pragma clang module begin B
+template<typename T> void f(const T&) { T::error; }
+#pragma clang module end
+#pragma clang module endbuild
+
+#pragma clang module build C
+module C {}
+#pragma clang module contents
+#pragma clang module begin C
+#pragma clang module load B
+template<typename T> void f(const T&) { T::error; }
+#pragma clang module end
+#pragma clang module endbuild
+
+#pragma clang module load A
+inline void f() {}
+void x() { f(); }
+
+#pragma clang module import C
+// expected-error@* {{cannot be used prior to}}
+void y(int n) { f(n); } // expected-note {{instantiation of}}