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