From: Richard Smith Date: Wed, 12 Sep 2018 23:37:00 +0000 (+0000) Subject: Track definition merging on the canonical declaration even when local X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7b26c1cf7574c1156ccaad5a34d23fadbf006a89;p=clang 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 0ed52ffd35..b107e8bd2a 100644 --- 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(ND->getCanonicalDecl())].push_back(M); - else - ND->setVisibleDespiteOwningModule(); + MergedDefModules[cast(ND->getCanonicalDecl())].push_back(M); } void ASTContext::deduplicateMergedDefinitonsFor(NamedDecl *ND) { diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index 900ccb18c7..819f9ef8a4 100644 --- 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 b60e5f14a6..ac3b737401 100644 --- 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 00fad25eea..8295107476 100644 --- 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(D); - if (auto *TD = dyn_cast(Exported)) - Exported = TD->getDefinition(); Module *Owner = SubmoduleID ? Reader.getSubmodule(SubmoduleID) : nullptr; - if (Reader.getContext().getLangOpts().ModulesLocalVisibility) { - Reader.getContext().mergeDefinitionIntoModule(cast(Exported), - Owner); - Reader.PendingMergedDefinitionsToDeduplicate.insert( - cast(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 index 0000000000..9ac1b6699d --- /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 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 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 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}}