r284176 - Reinstate r281429, reverted in r281452, with a fix for its mishandling of
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 13 16:04:15 PDT 2016
Author: rsmith
Date: Thu Oct 13 18:04:14 2016
New Revision: 284176
URL: http://llvm.org/viewvc/llvm-project?rev=284176&view=rev
Log:
Reinstate r281429, reverted in r281452, with a fix for its mishandling of
compiles without -fmodules-local-submodule-visibility. Original commit message:
[modules] When merging one definition into another, propagate the list of
re-exporting modules from the discarded definition to the retained definition.
Added:
cfe/trunk/test/Modules/Inputs/merge-var-template-def/
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=284176&r1=284175&r2=284176&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Thu Oct 13 18:04:14 2016
@@ -308,10 +308,18 @@ 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*, llvm::TinyPtrVector<Module*>> MergedDefModules;
+ llvm::DenseMap<NamedDecl*, MergedModulesOrCanonicalDef> 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
@@ -883,6 +891,7 @@ 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);
@@ -893,7 +902,9 @@ public:
auto MergedIt = MergedDefModules.find(Def);
if (MergedIt == MergedDefModules.end())
return None;
- return MergedIt->second;
+ if (auto *CanonDef = MergedIt->second.CanonicalDef)
+ return getModulesWithMergedDefinition(CanonDef);
+ return MergedIt->second.MergedModules;
}
/// 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=284176&r1=284175&r2=284176&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Thu Oct 13 18:04:14 2016
@@ -888,18 +888,72 @@ void ASTContext::mergeDefinitionIntoModu
if (auto *Listener = getASTMutationListener())
Listener->RedefinedHiddenDefinition(ND, M);
- if (getLangOpts().ModulesLocalVisibility)
- MergedDefModules[ND].push_back(M);
- else
+ 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)
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);
+ else
+ 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;
- auto &Merged = It->second;
+ if (auto *CanonDef = It->second.CanonicalDef) {
+ It = MergedDefModules.find(CanonDef);
+ if (It == MergedDefModules.end())
+ return;
+ }
+
+ auto &Merged = It->second.MergedModules;
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=284176&r1=284175&r2=284176&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Thu Oct 13 18:04:14 2016
@@ -3481,23 +3481,10 @@ 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.
- 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);
- }
+ getContext().mergeDefinitionIntoModulesOf(Def, MergedDef);
+ PendingMergedDefinitionsToDeduplicate.insert(Def);
}
}
@@ -8632,7 +8619,7 @@ void ASTReader::finishPendingActions() {
const FunctionDecl *Defn = nullptr;
if (!getContext().getLangOpts().Modules || !FD->hasBody(Defn))
FD->setLazyBody(PB->second);
- else
+ else if (FD != Defn)
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=284176&r1=284175&r2=284176&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 Thu Oct 13 18:04:14 2016
@@ -4,3 +4,8 @@ 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=284176&r1=284175&r2=284176&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 Thu Oct 13 18:04:14 2016
@@ -17,4 +17,7 @@ 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=284176&r1=284175&r2=284176&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 Thu Oct 13 18:04:14 2016
@@ -5,5 +5,8 @@ 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=284176&r1=284175&r2=284176&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 Thu Oct 13 18:04:14 2016
@@ -1 +1,3 @@
-// d.h: empty
+namespace CrossModuleMerge {
+ struct E {};
+}
Added: 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=284176&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-var-template-def/a.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-var-template-def/a.h Thu Oct 13 18:04:14 2016
@@ -0,0 +1,6 @@
+#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
Added: 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=284176&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-var-template-def/b1.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-var-template-def/b1.h Thu Oct 13 18:04:14 2016
@@ -0,0 +1,7 @@
+#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
Added: 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=284176&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-var-template-def/b2.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-var-template-def/b2.h Thu Oct 13 18:04:14 2016
@@ -0,0 +1,6 @@
+#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
Added: 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=284176&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-var-template-def/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/merge-var-template-def/module.modulemap Thu Oct 13 18:04:14 2016
@@ -0,0 +1,5 @@
+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=284176&r1=284175&r2=284176&view=diff
==============================================================================
--- cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp (original)
+++ cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp Thu Oct 13 18:04:14 2016
@@ -7,6 +7,12 @@
// 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"
@@ -15,4 +21,6 @@
void g() {
CrossModuleMerge::B<int> bi;
CrossModuleMerge::C(0);
+ CrossModuleMerge::D(0);
+ CrossModuleMerge::E e;
}
Added: 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=284176&view=auto
==============================================================================
--- cfe/trunk/test/Modules/merge-var-template-def.cpp (added)
+++ cfe/trunk/test/Modules/merge-var-template-def.cpp Thu Oct 13 18:04:14 2016
@@ -0,0 +1,7 @@
+// 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>();
More information about the cfe-commits
mailing list