r342097 - Track definition merging on the canonical declaration even when local
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 12 16:37:00 PDT 2018
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}}
More information about the cfe-commits
mailing list