I'm running into something else and your patch doesn't fix it. It boils down to 'std::is_nothrow_move_constructible<Foo>' from libstdc++ not compiling because Foo is not visible inside the noexcept specification of that template. I failed to come up with a small test case so far as it involves at least 2 modules :[
On Mon, Oct 17, 2016 at 4:44 PM, Vassil Vassilev <v.g.vassi...@gmail.com> wrote: > On 17/10/16 16:40, Benjamin Kramer wrote: >> >> Too slow ;) >> >> Do you have the fix somewhere, so I can try it? > > That would be https://reviews.llvm.org/D25678 > > Do you run into something else than what I have as a test case (merging > templated constexpr variables)? > >> >> 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
