r281258 - [modules] When we merge two definitions of a function, mark the retained
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 12 14:06:40 PDT 2016
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);
+}
More information about the cfe-commits
mailing list