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