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