From: Richard Smith Date: Mon, 12 Sep 2016 21:06:40 +0000 (+0000) Subject: [modules] When we merge two definitions of a function, mark the retained X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ba637c42b6b6451f52d1329ce45fd9c957d9e6f6;p=clang [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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@281258 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h index 810e869442..b05b419d23 100644 --- a/include/clang/Serialization/ASTReader.h +++ b/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 takeListener() { return std::move(Listener); diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index 7f48746d83..96a2693ade 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/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(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(Defn), FD); continue; } diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp index 70c7b7808c..d68d6d308a 100644 --- a/lib/Serialization/ASTReaderDecl.cpp +++ b/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/test/Modules/Inputs/merge-template-pattern-visibility/b.h b/test/Modules/Inputs/merge-template-pattern-visibility/b.h index 6db3c2c6c9..41b52d5e6a 100644 --- a/test/Modules/Inputs/merge-template-pattern-visibility/b.h +++ b/test/Modules/Inputs/merge-template-pattern-visibility/b.h @@ -9,3 +9,12 @@ inline void f() { B bi; C(0); } + +namespace CrossModuleMerge { + template struct A; + template struct B; + + template struct A {}; + template struct B : A {}; + template inline auto C(T) {} +} diff --git a/test/Modules/Inputs/merge-template-pattern-visibility/module.modulemap b/test/Modules/Inputs/merge-template-pattern-visibility/module.modulemap index ba97abbaa8..e00d1b9551 100644 --- a/test/Modules/Inputs/merge-template-pattern-visibility/module.modulemap +++ b/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/test/Modules/merge-template-pattern-visibility.cpp b/test/Modules/merge-template-pattern-visibility.cpp index b97f9f113e..ec5aa26c68 100644 --- a/test/Modules/merge-template-pattern-visibility.cpp +++ b/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 bi; + CrossModuleMerge::C(0); +}