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

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) {

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;
   };

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);
     }
   }
 }

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;
     }
 

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}}