Author: rsmith Date: Mon Sep 12 16:06:40 2016 New Revision: 281258 URL: http://llvm.org/viewvc/llvm-project?rev=281258&view=rev Log: [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.
Modified: cfe/trunk/include/clang/Serialization/ASTReader.h cfe/trunk/lib/Serialization/ASTReader.cpp cfe/trunk/lib/Serialization/ASTReaderDecl.cpp cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/module.modulemap cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp Modified: cfe/trunk/include/clang/Serialization/ASTReader.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=281258&r1=281257&r2=281258&view=diff ============================================================================== --- cfe/trunk/include/clang/Serialization/ASTReader.h (original) +++ cfe/trunk/include/clang/Serialization/ASTReader.h Mon Sep 12 16:06:40 2016 @@ -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); Modified: cfe/trunk/lib/Serialization/ASTReader.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=281258&r1=281257&r2=281258&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTReader.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReader.cpp Mon Sep 12 16:06:40 2016 @@ -3469,6 +3469,31 @@ void ASTReader::makeModuleVisible(Module } } +/// 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; } Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=281258&r1=281257&r2=281258&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Mon Sep 12 16:06:40 2016 @@ -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(EnumDe 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"); } Modified: cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h?rev=281258&r1=281257&r2=281258&view=diff ============================================================================== --- cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h (original) +++ cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h Mon Sep 12 16:06:40 2016 @@ -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) {} +} Modified: cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/module.modulemap URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/module.modulemap?rev=281258&r1=281257&r2=281258&view=diff ============================================================================== --- cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/module.modulemap (original) +++ cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/module.modulemap Mon Sep 12 16:06:40 2016 @@ -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" } +} Modified: cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp?rev=281258&r1=281257&r2=281258&view=diff ============================================================================== --- cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp (original) +++ cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp Mon Sep 12 16:06:40 2016 @@ -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); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits