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:44:06 PDT 2016
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.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