Too slow ;) Do you have the fix somewhere, so I can try it?
On Mon, Oct 17, 2016 at 4:38 PM, Vassil Vassilev <v.g.vassi...@gmail.com> wrote: > I was just to commit a fix :( > > On 17/10/16 15:00, Benjamin Kramer via cfe-commits wrote: >> >> Author: d0k >> Date: Mon Oct 17 08:00:44 2016 >> New Revision: 284382 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=284382&view=rev >> Log: >> Revert "Reinstate r281429, reverted in r281452, with a fix for its >> mishandling of" >> >> This reverts commit r284176. It still marks some modules as invisible >> that should be visible. Will follow up with the author with a test case. >> >> Removed: >> cfe/trunk/test/Modules/Inputs/merge-var-template-def/a.h >> cfe/trunk/test/Modules/Inputs/merge-var-template-def/b1.h >> cfe/trunk/test/Modules/Inputs/merge-var-template-def/b2.h >> cfe/trunk/test/Modules/Inputs/merge-var-template-def/module.modulemap >> cfe/trunk/test/Modules/merge-var-template-def.cpp >> 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/Inputs/merge-template-pattern-visibility/d.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=284382&r1=284381&r2=284382&view=diff >> >> ============================================================================== >> --- cfe/trunk/include/clang/AST/ASTContext.h (original) >> +++ cfe/trunk/include/clang/AST/ASTContext.h Mon Oct 17 08:00:44 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 >> @@ -891,7 +883,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); >> @@ -902,9 +893,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=284382&r1=284381&r2=284382&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/AST/ASTContext.cpp (original) >> +++ cfe/trunk/lib/AST/ASTContext.cpp Mon Oct 17 08:00:44 2016 >> @@ -888,74 +888,18 @@ void ASTContext::mergeDefinitionIntoModu >> if (auto *Listener = getASTMutationListener()) >> Listener->RedefinedHiddenDefinition(ND, M); >> - auto *Merged = &MergedDefModules[ND]; >> - if (auto *CanonDef = Merged->CanonicalDef) { >> - ND = CanonDef; >> - Merged = &MergedDefModules[ND]; >> - } >> - assert(!Merged->CanonicalDef && "canonical def not canonical"); >> - >> - Merged->MergedModules.push_back(M); >> - >> - if (!getLangOpts().ModulesLocalVisibility) >> + 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 (!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) { >> auto It = MergedDefModules.find(ND); >> 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=284382&r1=284381&r2=284382&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original) >> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Mon Oct 17 08:00:44 2016 >> @@ -3481,10 +3481,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); >> + } >> } >> } >> @@ -8619,7 +8632,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=284382&r1=284381&r2=284382&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 >> Mon Oct 17 08:00:44 2016 >> @@ -4,8 +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) {} >> - struct E {}; >> -} >> >> 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=284382&r1=284381&r2=284382&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 Oct 17 08:00:44 2016 >> @@ -17,7 +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) {} >> - struct E; >> } >> >> 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=284382&r1=284381&r2=284382&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 >> Mon Oct 17 08:00:44 2016 >> @@ -5,8 +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) {} >> - struct E {}; >> } >> >> Modified: >> cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/d.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/d.h?rev=284382&r1=284381&r2=284382&view=diff >> >> ============================================================================== >> --- cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/d.h >> (original) >> +++ cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/d.h >> Mon Oct 17 08:00:44 2016 >> @@ -1,3 +1 @@ >> -namespace CrossModuleMerge { >> - struct E {}; >> -} >> +// d.h: empty >> >> Removed: cfe/trunk/test/Modules/Inputs/merge-var-template-def/a.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-var-template-def/a.h?rev=284381&view=auto >> >> ============================================================================== >> --- cfe/trunk/test/Modules/Inputs/merge-var-template-def/a.h (original) >> +++ cfe/trunk/test/Modules/Inputs/merge-var-template-def/a.h (removed) >> @@ -1,6 +0,0 @@ >> -#ifndef A_H >> -#define A_H >> -template<typename T> struct A { static bool b; }; >> -template<typename T> bool A<T>::b = false; >> -template<typename T> void *get() { return &(A<T>::b); } >> -#endif >> >> Removed: cfe/trunk/test/Modules/Inputs/merge-var-template-def/b1.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-var-template-def/b1.h?rev=284381&view=auto >> >> ============================================================================== >> --- cfe/trunk/test/Modules/Inputs/merge-var-template-def/b1.h (original) >> +++ cfe/trunk/test/Modules/Inputs/merge-var-template-def/b1.h (removed) >> @@ -1,7 +0,0 @@ >> -#ifndef B1_H >> -#define B1_H >> -template<typename T> struct A { static bool b; }; >> -template<typename T> bool A<T>::b = false; >> -template<typename T> void *get() { return &(A<T>::b); } >> -#include "a.h" >> -#endif >> >> Removed: cfe/trunk/test/Modules/Inputs/merge-var-template-def/b2.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-var-template-def/b2.h?rev=284381&view=auto >> >> ============================================================================== >> --- cfe/trunk/test/Modules/Inputs/merge-var-template-def/b2.h (original) >> +++ cfe/trunk/test/Modules/Inputs/merge-var-template-def/b2.h (removed) >> @@ -1,6 +0,0 @@ >> -#ifndef B2_H >> -#define B2_H >> -template<typename T> struct A { static bool b; }; >> -template<typename T> bool A<T>::b = false; >> -template<typename T> void *get() { return &(A<T>::b); } >> -#endif >> >> Removed: >> cfe/trunk/test/Modules/Inputs/merge-var-template-def/module.modulemap >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-var-template-def/module.modulemap?rev=284381&view=auto >> >> ============================================================================== >> --- cfe/trunk/test/Modules/Inputs/merge-var-template-def/module.modulemap >> (original) >> +++ cfe/trunk/test/Modules/Inputs/merge-var-template-def/module.modulemap >> (removed) >> @@ -1,5 +0,0 @@ >> -module a { header "a.h" export * } >> -module b { >> - module b1 { header "b1.h" export * } >> - module b2 { header "b2.h" export * } >> -} >> >> 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=284382&r1=284381&r2=284382&view=diff >> >> ============================================================================== >> --- cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp >> (original) >> +++ cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp Mon Oct >> 17 08:00:44 2016 >> @@ -7,12 +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 >> -// RUN: %clang_cc1 -fmodules -fno-modules-error-recovery -std=c++14 >> -fmodule-file=%t/X.pcm -fmodule-file=%t/Y.pcm \ >> -// RUN: -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: -verify %s >> -I%S/Inputs/merge-template-pattern-visibility >> #include "b.h" >> #include "d.h" >> @@ -21,6 +15,4 @@ >> void g() { >> CrossModuleMerge::B<int> bi; >> CrossModuleMerge::C(0); >> - CrossModuleMerge::D(0); >> - CrossModuleMerge::E e; >> } >> >> Removed: cfe/trunk/test/Modules/merge-var-template-def.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-var-template-def.cpp?rev=284381&view=auto >> >> ============================================================================== >> --- cfe/trunk/test/Modules/merge-var-template-def.cpp (original) >> +++ cfe/trunk/test/Modules/merge-var-template-def.cpp (removed) >> @@ -1,7 +0,0 @@ >> -// RUN: rm -rf %t >> -// RUN: %clang_cc1 -I%S/Inputs/merge-var-template-def -verify -fmodules >> -Werror=undefined-internal -fmodules-local-submodule-visibility >> -fmodules-cache-path=%t -fimplicit-module-maps %s >> -// expected-no-diagnostics >> - >> -#include "b2.h" >> -namespace { struct X; } >> -void *x = get<X>(); >> >> >> _______________________________________________ >> 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