From b19f8fa7328ea6963987cc52ecf46cde361cb181 Mon Sep 17 00:00:00 2001
From: Eric Liu <ioeric@google.com>
Date: Wed, 14 Sep 2016 10:05:10 +0000
Subject: [PATCH] Revert "[modules] When merging one definition into another,
propagate the list of re-exporting modules from the discarded definition to
the retained definition."
This reverts commit r281429.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@281452 91177308-0d34-0410-b5e6-96231b3b80d8
---
include/clang/AST/ASTContext.h | 15 +----
lib/AST/ASTContext.cpp | 63 ++-----------------
lib/Serialization/ASTReader.cpp | 19 +++++-
.../merge-template-pattern-visibility/a.h | 4 --
.../merge-template-pattern-visibility/b.h | 2 -
.../merge-template-pattern-visibility/c.h | 2 -
.../merge-template-pattern-visibility.cpp | 3 -
7 files changed, 22 insertions(+), 86 deletions(-)
diff --git a/include/clang/AST/ASTContext.h b/include/clang/AST/ASTContext.h
index 0bc4aa4d80f..0a184bc2e37 100644
--- a/include/clang/AST/ASTContext.h
+++ b/include/clang/AST/ASTContext.h
@@ -308,18 +308,10 @@ class ASTContext : public RefCountedBase<ASTContext> {
/// 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
@@ -902,7 +894,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);
@@ -913,9 +904,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
diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp
index e5d7ef244fb..9e801b5d7ae 100644
--- a/lib/AST/ASTContext.cpp
+++ b/lib/AST/ASTContext.cpp
@@ -890,59 +890,10 @@ void ASTContext::mergeDefinitionIntoModule(NamedDecl *ND, Module *M,
if (auto *Listener = getASTMutationListener())
Listener->RedefinedHiddenDefinition(ND, M);
- if (getLangOpts().ModulesLocalVisibility) {
- auto *Merged = &MergedDefModules[ND];
- if (auto *CanonDef = Merged->CanonicalDef)
- Merged = &MergedDefModules[CanonDef];
- Merged->MergedModules.push_back(M);
- } else {
- auto MergedIt = MergedDefModules.find(ND);
- if (MergedIt != MergedDefModules.end() && MergedIt->second.CanonicalDef)
- ND = MergedIt->second.CanonicalDef;
+ 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 (!getLangOpts().ModulesLocalVisibility && !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) {
@@ -950,13 +901,7 @@ void ASTContext::deduplicateMergedDefinitonsFor(NamedDecl *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)
diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index a46d2d21d2f..9b51f3dae52 100644
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -3474,10 +3474,23 @@ void ASTReader::makeModuleVisible(Module *Mod,
/// 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);
+ }
}
}
@@ -8608,7 +8621,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;
}
diff --git a/test/Modules/Inputs/merge-template-pattern-visibility/a.h b/test/Modules/Inputs/merge-template-pattern-visibility/a.h
index 7225110a377..e6592027611 100644
--- a/test/Modules/Inputs/merge-template-pattern-visibility/a.h
+++ b/test/Modules/Inputs/merge-template-pattern-visibility/a.h
@@ -4,7 +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) {}
-}
diff --git a/test/Modules/Inputs/merge-template-pattern-visibility/b.h b/test/Modules/Inputs/merge-template-pattern-visibility/b.h
index 7301ca722dd..41b52d5e6ab 100644
--- a/test/Modules/Inputs/merge-template-pattern-visibility/b.h
+++ b/test/Modules/Inputs/merge-template-pattern-visibility/b.h
@@ -17,6 +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) {}
}
diff --git a/test/Modules/Inputs/merge-template-pattern-visibility/c.h b/test/Modules/Inputs/merge-template-pattern-visibility/c.h
index 68957dbad07..db80eda1ea6 100644
--- a/test/Modules/Inputs/merge-template-pattern-visibility/c.h
+++ b/test/Modules/Inputs/merge-template-pattern-visibility/c.h
@@ -5,7 +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) {}
}
diff --git a/test/Modules/merge-template-pattern-visibility.cpp b/test/Modules/merge-template-pattern-visibility.cpp
index 4cfac176c05..ec5aa26c68d 100644
--- a/test/Modules/merge-template-pattern-visibility.cpp
+++ b/test/Modules/merge-template-pattern-visibility.cpp
@@ -7,8 +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
#include "b.h"
#include "d.h"
@@ -17,5 +15,4 @@
void g() {
CrossModuleMerge::B<int> bi;
CrossModuleMerge::C(0);
- CrossModuleMerge::D(0);
}
--
GitLab