[modules] When we merge two definitions of a function, mark the retained

definition as visible in the discarded definition's module, as we do for
other kinds of definition.

llvm-svn: 281258

diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 810e869..b05b419 100644 (file)
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1420,6 +1420,10 @@ public:
   /// \brief Make the names within this set of hidden names visible.
   void makeNamesVisible(const HiddenNames &Names, Module *Owner);
 
+  /// \brief Note that MergedDef is a redefinition of the canonical definition
+  /// Def, so Def should be visible whenever MergedDef is.
+  void mergeDefinitionVisibility(NamedDecl *Def, NamedDecl *MergedDef);
+
   /// \brief Take the AST callbacks listener.
   std::unique_ptr<ASTReaderListener> takeListener() {
     return std::move(Listener);

diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 7f48746..96a2693 100644 (file)
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -3469,6 +3469,31 @@ void ASTReader::makeModuleVisible(Module *Mod,
   }
 }
 
+/// We've merged the definition \p MergedDef into the existing definition
+/// \p Def. Ensure that \p Def is made visible whenever \p MergedDef is made
+/// 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->Hidden = false;
+    else if (getContext().getLangOpts().ModulesLocalVisibility) {
+      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);
+    }
+  }
+}
+
 bool ASTReader::loadGlobalIndex() {
   if (GlobalIndex)
     return false;
@@ -8574,8 +8599,11 @@ void ASTReader::finishPendingActions() {
     if (FunctionDecl *FD = dyn_cast<FunctionDecl>(PB->first)) {
       // FIXME: Check for =delete/=default?
       // FIXME: Complain about ODR violations here?
-      if (!getContext().getLangOpts().Modules || !FD->hasBody())
+      const FunctionDecl *Defn = nullptr;
+      if (!getContext().getLangOpts().Modules || !FD->hasBody(Defn))
         FD->setLazyBody(PB->second);
+      else
+        mergeDefinitionVisibility(const_cast<FunctionDecl*>(Defn), FD);
       continue;
     }
 

diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 70c7b78..d68d6d3 100644 (file)
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -383,27 +383,6 @@ namespace clang {
     void VisitOMPThreadPrivateDecl(OMPThreadPrivateDecl *D);
     void VisitOMPDeclareReductionDecl(OMPDeclareReductionDecl *D);
     void VisitOMPCapturedExprDecl(OMPCapturedExprDecl *D);
-
-    /// We've merged the definition \p MergedDef into the existing definition
-    /// \p Def. Ensure that \p Def is made visible whenever \p MergedDef is made
-    /// visible.
-    void mergeDefinitionVisibility(NamedDecl *Def, NamedDecl *MergedDef) {
-      if (Def->isHidden()) {
-        // If MergedDef is visible or becomes visible, make the definition visible.
-        if (!MergedDef->isHidden())
-          Def->Hidden = false;
-        else if (Reader.getContext().getLangOpts().ModulesLocalVisibility) {
-          Reader.getContext().mergeDefinitionIntoModule(
-              Def, MergedDef->getImportedOwningModule(),
-              /*NotifyListeners*/ false);
-          Reader.PendingMergedDefinitionsToDeduplicate.insert(Def);
-        } else {
-          auto SubmoduleID = MergedDef->getOwningModuleID();
-          assert(SubmoduleID && "hidden definition in no module");
-          Reader.HiddenNamesMap[Reader.getSubmodule(SubmoduleID)].push_back(Def);
-        }
-      }
-    }
   };
 } // end namespace clang
 
@@ -710,7 +689,7 @@ void ASTDeclReader::VisitEnumDecl(EnumDecl *ED) {
     if (OldDef) {
       Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef));
       ED->IsCompleteDefinition = false;
-      mergeDefinitionVisibility(OldDef, ED);
+      Reader.mergeDefinitionVisibility(OldDef, ED);
     } else {
       OldDef = ED;
     }
@@ -1603,7 +1582,7 @@ void ASTDeclReader::MergeDefinitionData(
                                                     DD.Definition));
     Reader.PendingDefinitions.erase(MergeDD.Definition);
     MergeDD.Definition->IsCompleteDefinition = false;
-    mergeDefinitionVisibility(DD.Definition, MergeDD.Definition);
+    Reader.mergeDefinitionVisibility(DD.Definition, MergeDD.Definition);
     assert(Reader.Lookups.find(MergeDD.Definition) == Reader.Lookups.end() &&
            "already loaded pending lookups for merged definition");
   }

diff --git a/clang/test/Modules/Inputs/merge-template-pattern-visibility/b.h b/clang/test/Modules/Inputs/merge-template-pattern-visibility/b.h
index 6db3c2c..41b52d5 100644 (file)
--- a/clang/test/Modules/Inputs/merge-template-pattern-visibility/b.h
+++ b/clang/test/Modules/Inputs/merge-template-pattern-visibility/b.h
@@ -9,3 +9,12 @@ inline void f() {
   B<int> bi;
   C(0);
 }
+
+namespace CrossModuleMerge {
+  template<typename, typename = int> struct A;
+  template<typename T> struct B;
+
+  template<typename, typename> struct A {};
+  template<typename T> struct B : A<T> {};
+  template<typename T> inline auto C(T) {}
+}

diff --git a/clang/test/Modules/Inputs/merge-template-pattern-visibility/module.modulemap b/clang/test/Modules/Inputs/merge-template-pattern-visibility/module.modulemap
index ba97abb..e00d1b9 100644 (file)
--- a/clang/test/Modules/Inputs/merge-template-pattern-visibility/module.modulemap
+++ b/clang/test/Modules/Inputs/merge-template-pattern-visibility/module.modulemap
@@ -2,3 +2,7 @@ module X {
   module A { header "a.h" }
   module B { header "b.h" }
 }
+module Y {
+  module C { header "c.h" }
+  module D { header "d.h" }
+}

diff --git a/clang/test/Modules/merge-template-pattern-visibility.cpp b/clang/test/Modules/merge-template-pattern-visibility.cpp
index b97f9f1..ec5aa26 100644 (file)
--- a/clang/test/Modules/merge-template-pattern-visibility.cpp
+++ b/clang/test/Modules/merge-template-pattern-visibility.cpp
@@ -2,3 +2,17 @@
 // RUN: %clang_cc1 -fmodules -fno-modules-error-recovery -std=c++14 \
 // RUN:            -fmodule-name=X -emit-module %S/Inputs/merge-template-pattern-visibility/module.modulemap -x c++ \
 // RUN:            -fmodules-local-submodule-visibility -o %t/X.pcm
+// RUN: %clang_cc1 -fmodules -fno-modules-error-recovery -std=c++14 \
+// RUN:            -fmodule-name=Y -emit-module %S/Inputs/merge-template-pattern-visibility/module.modulemap -x c++ \
+// RUN:            -fmodules-local-submodule-visibility -o %t/Y.pcm
+// RUN: %clang_cc1 -fmodules -fno-modules-error-recovery -std=c++14 -fmodule-file=%t/X.pcm -fmodule-file=%t/Y.pcm \
+// RUN:            -fmodules-local-submodule-visibility -verify %s -I%S/Inputs/merge-template-pattern-visibility
+
+#include "b.h"
+#include "d.h"
+
+// expected-no-diagnostics
+void g() {
+  CrossModuleMerge::B<int> bi;
+  CrossModuleMerge::C(0);
+}