<p dir="ltr">When reverting something, please say why in the commit message.</p>
<div class="gmail_extra"><br><div class="gmail_quote">On Sep 14, 2016 6:13 AM, "Eric Liu via cfe-commits" <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: ioeric<br>
Date: Wed Sep 14 05:05:10 2016<br>
New Revision: 281452<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=281452&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=281452&view=rev</a><br>
Log:<br>
Revert "[modules] When merging one definition into another, propagate the list of re-exporting modules from the discarded definition to the retained definition."<br>
<br>
This reverts commit r281429.<br>
<br>
Modified:<br>
cfe/trunk/include/clang/AST/<wbr>ASTContext.h<br>
cfe/trunk/lib/AST/ASTContext.<wbr>cpp<br>
cfe/trunk/lib/Serialization/<wbr>ASTReader.cpp<br>
cfe/trunk/test/Modules/Inputs/<wbr>merge-template-pattern-<wbr>visibility/a.h<br>
cfe/trunk/test/Modules/Inputs/<wbr>merge-template-pattern-<wbr>visibility/b.h<br>
cfe/trunk/test/Modules/Inputs/<wbr>merge-template-pattern-<wbr>visibility/c.h<br>
cfe/trunk/test/Modules/merge-<wbr>template-pattern-visibility.<wbr>cpp<br>
<br>
Modified: cfe/trunk/include/clang/AST/<wbr>ASTContext.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=281452&r1=281451&r2=281452&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/include/<wbr>clang/AST/ASTContext.h?rev=<wbr>281452&r1=281451&r2=281452&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/include/clang/AST/<wbr>ASTContext.h (original)<br>
+++ cfe/trunk/include/clang/AST/<wbr>ASTContext.h Wed Sep 14 05:05:10 2016<br>
@@ -308,18 +308,10 @@ class ASTContext : public RefCountedBase<br>
/// merged into.<br>
llvm::DenseMap<Decl*, Decl*> MergedDecls;<br>
<br>
- /// The modules into which a definition has been merged, or a map from a<br>
- /// merged definition to its canonical definition. This is really a union of<br>
- /// a NamedDecl* and a vector of Module*.<br>
- struct MergedModulesOrCanonicalDef {<br>
- llvm::TinyPtrVector<Module*> MergedModules;<br>
- NamedDecl *CanonicalDef = nullptr;<br>
- };<br>
-<br>
/// \brief A mapping from a defining declaration to a list of modules (other<br>
/// than the owning module of the declaration) that contain merged<br>
/// definitions of that entity.<br>
- llvm::DenseMap<NamedDecl*, MergedModulesOrCanonicalDef> MergedDefModules;<br>
+ llvm::DenseMap<NamedDecl*, llvm::TinyPtrVector<Module*>> MergedDefModules;<br>
<br>
/// \brief Initializers for a module, in order. Each Decl will be either<br>
/// something that has a semantic effect on startup (such as a variable with<br>
@@ -902,7 +894,6 @@ public:<br>
/// and should be visible whenever \p M is visible.<br>
void mergeDefinitionIntoModule(<wbr>NamedDecl *ND, Module *M,<br>
bool NotifyListeners = true);<br>
- void mergeDefinitionIntoModulesOf(<wbr>NamedDecl *ND, NamedDecl *Other);<br>
/// \brief Clean up the merged definition list. Call this if you might have<br>
/// added duplicates into the list.<br>
void deduplicateMergedDefinitonsFor<wbr>(NamedDecl *ND);<br>
@@ -913,9 +904,7 @@ public:<br>
auto MergedIt = MergedDefModules.find(Def);<br>
if (MergedIt == MergedDefModules.end())<br>
return None;<br>
- if (auto *CanonDef = MergedIt->second.CanonicalDef)<br>
- return getModulesWithMergedDefinition<wbr>(CanonDef);<br>
- return MergedIt->second.<wbr>MergedModules;<br>
+ return MergedIt->second;<br>
}<br>
<br>
/// Add a declaration to the list of declarations that are initialized<br>
<br>
Modified: cfe/trunk/lib/AST/ASTContext.<wbr>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=281452&r1=281451&r2=281452&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/AST/<wbr>ASTContext.cpp?rev=281452&r1=<wbr>281451&r2=281452&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/AST/ASTContext.<wbr>cpp (original)<br>
+++ cfe/trunk/lib/AST/ASTContext.<wbr>cpp Wed Sep 14 05:05:10 2016<br>
@@ -890,59 +890,10 @@ void ASTContext::<wbr>mergeDefinitionIntoModu<br>
if (auto *Listener = getASTMutationListener())<br>
Listener-><wbr>RedefinedHiddenDefinition(ND, M);<br>
<br>
- if (getLangOpts().<wbr>ModulesLocalVisibility) {<br>
- auto *Merged = &MergedDefModules[ND];<br>
- if (auto *CanonDef = Merged->CanonicalDef)<br>
- Merged = &MergedDefModules[CanonDef];<br>
- Merged->MergedModules.push_<wbr>back(M);<br>
- } else {<br>
- auto MergedIt = MergedDefModules.find(ND);<br>
- if (MergedIt != MergedDefModules.end() && MergedIt->second.CanonicalDef)<br>
- ND = MergedIt->second.CanonicalDef;<br>
+ if (getLangOpts().<wbr>ModulesLocalVisibility)<br>
+ MergedDefModules[ND].push_<wbr>back(M);<br>
+ else<br>
ND->setHidden(false);<br>
- }<br>
-}<br>
-<br>
-void ASTContext::<wbr>mergeDefinitionIntoModulesOf(<wbr>NamedDecl *Def,<br>
- NamedDecl *Other) {<br>
- // We need to know the owning module of the merge source.<br>
- assert(Other->isFromASTFile() && "merge of non-imported decl not supported");<br>
- assert(Def != Other && "merging definition into itself");<br>
-<br>
- if (!getLangOpts().<wbr>ModulesLocalVisibility && !Other->isHidden()) {<br>
- Def->setHidden(false);<br>
- return;<br>
- }<br>
- assert(Other-><wbr>getImportedOwningModule() &&<br>
- "hidden, imported declaration has no owning module");<br>
-<br>
- // Mark Def as the canonical definition of merged definition Other.<br>
- {<br>
- auto &OtherMerged = MergedDefModules[Other];<br>
- assert((!OtherMerged.<wbr>CanonicalDef || OtherMerged.CanonicalDef == Def) &&<br>
- "mismatched canonical definitions for declaration");<br>
- OtherMerged.CanonicalDef = Def;<br>
- }<br>
-<br>
- auto &Merged = MergedDefModules[Def];<br>
- // Grab this again, we potentially just invalidated our reference.<br>
- auto &OtherMerged = MergedDefModules[Other];<br>
-<br>
- if (Module *M = Other-><wbr>getImportedOwningModule())<br>
- Merged.MergedModules.push_<wbr>back(M);<br>
-<br>
- // If this definition had any others merged into it, they're now merged into<br>
- // the canonical definition instead.<br>
- if (!OtherMerged.MergedModules.<wbr>empty()) {<br>
- assert(!Merged.CanonicalDef && "canonical definition not canonical");<br>
- if (Merged.MergedModules.empty())<br>
- Merged.MergedModules = std::move(OtherMerged.<wbr>MergedModules);<br>
- else<br>
- Merged.MergedModules.insert(<wbr>Merged.MergedModules.end(),<br>
- OtherMerged.MergedModules.<wbr>begin(),<br>
- OtherMerged.MergedModules.end(<wbr>));<br>
- OtherMerged.MergedModules.<wbr>clear();<br>
- }<br>
}<br>
<br>
void ASTContext::<wbr>deduplicateMergedDefinitonsFor<wbr>(NamedDecl *ND) {<br>
@@ -950,13 +901,7 @@ void ASTContext::<wbr>deduplicateMergedDefini<br>
if (It == MergedDefModules.end())<br>
return;<br>
<br>
- if (auto *CanonDef = It->second.CanonicalDef) {<br>
- It = MergedDefModules.find(<wbr>CanonDef);<br>
- if (It == MergedDefModules.end())<br>
- return;<br>
- }<br>
-<br>
- auto &Merged = It->second.MergedModules;<br>
+ auto &Merged = It->second;<br>
llvm::DenseSet<Module*> Found;<br>
for (Module *&M : Merged)<br>
if (!Found.insert(M).second)<br>
<br>
Modified: cfe/trunk/lib/Serialization/<wbr>ASTReader.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=281452&r1=281451&r2=281452&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/<wbr>Serialization/ASTReader.cpp?<wbr>rev=281452&r1=281451&r2=<wbr>281452&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Serialization/<wbr>ASTReader.cpp (original)<br>
+++ cfe/trunk/lib/Serialization/<wbr>ASTReader.cpp Wed Sep 14 05:05:10 2016<br>
@@ -3474,10 +3474,23 @@ void ASTReader::makeModuleVisible(<wbr>Module<br>
/// visible.<br>
void ASTReader::<wbr>mergeDefinitionVisibility(<wbr>NamedDecl *Def,<br>
NamedDecl *MergedDef) {<br>
+ // FIXME: This doesn't correctly handle the case where MergedDef is visible<br>
+ // in modules other than its owning module. We should instead give the<br>
+ // ASTContext a list of merged definitions for Def.<br>
if (Def->isHidden()) {<br>
// If MergedDef is visible or becomes visible, make the definition visible.<br>
- getContext().<wbr>mergeDefinitionIntoModulesOf(<wbr>Def, MergedDef);<br>
- PendingMergedDefinitionsToDedu<wbr>plicate.insert(Def);<br>
+ if (!MergedDef->isHidden())<br>
+ Def->Hidden = false;<br>
+ else if (getContext().getLangOpts().<wbr>ModulesLocalVisibility) {<br>
+ getContext().<wbr>mergeDefinitionIntoModule(<br>
+ Def, MergedDef-><wbr>getImportedOwningModule(),<br>
+ /*NotifyListeners*/ false);<br>
+ PendingMergedDefinitionsToDedu<wbr>plicate.insert(Def);<br>
+ } else {<br>
+ auto SubmoduleID = MergedDef->getOwningModuleID()<wbr>;<br>
+ assert(SubmoduleID && "hidden definition in no module");<br>
+ HiddenNamesMap[getSubmodule(<wbr>SubmoduleID)].push_back(Def);<br>
+ }<br>
}<br>
}<br>
<br>
@@ -8608,7 +8621,7 @@ void ASTReader::<wbr>finishPendingActions() {<br>
const FunctionDecl *Defn = nullptr;<br>
if (!getContext().getLangOpts().<wbr>Modules || !FD->hasBody(Defn))<br>
FD->setLazyBody(PB->second);<br>
- else if (FD != Defn)<br>
+ else<br>
mergeDefinitionVisibility(<wbr>const_cast<FunctionDecl*>(<wbr>Defn), FD);<br>
continue;<br>
}<br>
<br>
Modified: cfe/trunk/test/Modules/Inputs/<wbr>merge-template-pattern-<wbr>visibility/a.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h?rev=281452&r1=281451&r2=281452&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/<wbr>Modules/Inputs/merge-template-<wbr>pattern-visibility/a.h?rev=<wbr>281452&r1=281451&r2=281452&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Modules/Inputs/<wbr>merge-template-pattern-<wbr>visibility/a.h (original)<br>
+++ cfe/trunk/test/Modules/Inputs/<wbr>merge-template-pattern-<wbr>visibility/a.h Wed Sep 14 05:05:10 2016<br>
@@ -4,7 +4,3 @@ template<typename T> struct B;<br>
template<typename, typename> struct A {};<br>
template<typename T> struct B : A<T> {};<br>
template<typename T> inline auto C(T) {}<br>
-<br>
-namespace CrossModuleMerge {<br>
- template<typename T> inline auto D(T) {}<br>
-}<br>
<br>
Modified: cfe/trunk/test/Modules/Inputs/<wbr>merge-template-pattern-<wbr>visibility/b.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h?rev=281452&r1=281451&r2=281452&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/<wbr>Modules/Inputs/merge-template-<wbr>pattern-visibility/b.h?rev=<wbr>281452&r1=281451&r2=281452&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Modules/Inputs/<wbr>merge-template-pattern-<wbr>visibility/b.h (original)<br>
+++ cfe/trunk/test/Modules/Inputs/<wbr>merge-template-pattern-<wbr>visibility/b.h Wed Sep 14 05:05:10 2016<br>
@@ -17,6 +17,4 @@ namespace CrossModuleMerge {<br>
template<typename, typename> struct A {};<br>
template<typename T> struct B : A<T> {};<br>
template<typename T> inline auto C(T) {}<br>
-<br>
- template<typename T> inline auto D(T) {}<br>
}<br>
<br>
Modified: cfe/trunk/test/Modules/Inputs/<wbr>merge-template-pattern-<wbr>visibility/c.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/c.h?rev=281452&r1=281451&r2=281452&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/<wbr>Modules/Inputs/merge-template-<wbr>pattern-visibility/c.h?rev=<wbr>281452&r1=281451&r2=281452&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Modules/Inputs/<wbr>merge-template-pattern-<wbr>visibility/c.h (original)<br>
+++ cfe/trunk/test/Modules/Inputs/<wbr>merge-template-pattern-<wbr>visibility/c.h Wed Sep 14 05:05:10 2016<br>
@@ -5,7 +5,5 @@ namespace CrossModuleMerge {<br>
template<typename, typename> struct A {};<br>
template<typename T> struct B : A<T> {};<br>
template<typename T> inline auto C(T) {}<br>
-<br>
- template<typename T> inline auto D(T) {}<br>
}<br>
<br>
<br>
Modified: cfe/trunk/test/Modules/merge-<wbr>template-pattern-visibility.<wbr>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp?rev=281452&r1=281451&r2=281452&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/<wbr>Modules/merge-template-<wbr>pattern-visibility.cpp?rev=<wbr>281452&r1=281451&r2=281452&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Modules/merge-<wbr>template-pattern-visibility.<wbr>cpp (original)<br>
+++ cfe/trunk/test/Modules/merge-<wbr>template-pattern-visibility.<wbr>cpp Wed Sep 14 05:05:10 2016<br>
@@ -7,8 +7,6 @@<br>
// RUN: -fmodules-local-submodule-<wbr>visibility -o %t/Y.pcm<br>
// RUN: %clang_cc1 -fmodules -fno-modules-error-recovery -std=c++14 -fmodule-file=%t/X.pcm -fmodule-file=%t/Y.pcm \<br>
// RUN: -fmodules-local-submodule-<wbr>visibility -verify %s -I%S/Inputs/merge-template-<wbr>pattern-visibility<br>
-// RUN: %clang_cc1 -fmodules -fno-modules-error-recovery -std=c++14 -fmodule-file=%t/Y.pcm -fmodule-file=%t/X.pcm \<br>
-// RUN: -fmodules-local-submodule-<wbr>visibility -verify %s -I%S/Inputs/merge-template-<wbr>pattern-visibility<br>
<br>
#include "b.h"<br>
#include "d.h"<br>
@@ -17,5 +15,4 @@<br>
void g() {<br>
CrossModuleMerge::B<int> bi;<br>
CrossModuleMerge::C(0);<br>
- CrossModuleMerge::D(0);<br>
}<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div>