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