Author: rsmith Date: Wed Sep 12 16:37:00 2018 New Revision: 342097 URL: http://llvm.org/viewvc/llvm-project?rev=342097&view=rev Log: 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. Added: cfe/trunk/test/Modules/merge-template-pattern-visibility-3.cpp Modified: cfe/trunk/lib/AST/ASTContext.cpp cfe/trunk/lib/Sema/SemaType.cpp cfe/trunk/lib/Serialization/ASTReader.cpp cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Modified: cfe/trunk/lib/AST/ASTContext.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=342097&r1=342096&r2=342097&view=diff ============================================================================== --- cfe/trunk/lib/AST/ASTContext.cpp (original) +++ cfe/trunk/lib/AST/ASTContext.cpp Wed Sep 12 16:37:00 2018 @@ -932,10 +932,7 @@ void ASTContext::mergeDefinitionIntoModu 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) { Modified: cfe/trunk/lib/Sema/SemaType.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=342097&r1=342096&r2=342097&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaType.cpp (original) +++ cfe/trunk/lib/Sema/SemaType.cpp Wed Sep 12 16:37:00 2018 @@ -7623,8 +7623,15 @@ bool Sema::hasVisibleDefinition(NamedDec // 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; }; Modified: cfe/trunk/lib/Serialization/ASTReader.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=342097&r1=342096&r2=342097&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTReader.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Sep 12 16:37:00 2018 @@ -3780,22 +3780,15 @@ void ASTReader::makeModuleVisible(Module /// 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); } } } Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=342097&r1=342096&r2=342097&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Sep 12 16:37:00 2018 @@ -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; } Added: cfe/trunk/test/Modules/merge-template-pattern-visibility-3.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-template-pattern-visibility-3.cpp?rev=342097&view=auto ============================================================================== --- cfe/trunk/test/Modules/merge-template-pattern-visibility-3.cpp (added) +++ cfe/trunk/test/Modules/merge-template-pattern-visibility-3.cpp Wed Sep 12 16:37:00 2018 @@ -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}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits