r284382 - Revert "Reinstate r281429, reverted in r281452, with a fix for its mishandling of"
Vassil Vassilev via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 17 07:38:26 PDT 2016
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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list