Author: rsmith
Date: Wed Jul 5 00:47:11 2017
New Revision: 307129
URL: http://llvm.org/viewvc/llvm-project?rev=307129&view=rev
Log:
[modules ts] Improve merging of module-private declarations.
These cases occur frequently for declarations in the global module (above the
module-declaration) in a Modules TS module interface. When we merge a
definition from another module into such a module-private definition, ensure
that we transitively make everything lexically within that definition visible
to that translation unit.
Added:
cfe/trunk/test/CXX/modules-ts/dcl.dcl/dcl.module/p5.cpp
Modified:
cfe/trunk/include/clang/AST/DeclBase.h
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=307129&r1=307128&r2=307129&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Wed Jul 5 00:47:11 2017
@@ -749,7 +749,7 @@ public:
/// Set that this declaration is globally visible, even if it came from a
/// module that is not visible.
void setVisibleDespiteOwningModule() {
- if (getModuleOwnershipKind() == ModuleOwnershipKind::VisibleWhenImported)
+ if (isHidden())
setModuleOwnershipKind(ModuleOwnershipKind::Visible);
}
Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=307129&r1=307128&r2=307129&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Wed Jul 5 00:47:11 2017
@@ -283,8 +283,10 @@ void Decl::setLexicalDeclContext(DeclCon
setLocalOwningModule(cast<Decl>(DC)->getOwningModule());
}
- assert((!hasOwningModule() || getOwningModule() || isModulePrivate()) &&
- "hidden declaration has no owning module");
+ assert(
+ (getModuleOwnershipKind() != ModuleOwnershipKind::VisibleWhenImported ||
+ getOwningModule()) &&
+ "hidden declaration has no owning module");
}
void Decl::setDeclContextsImpl(DeclContext *SemaDC, DeclContext *LexicalDC,
Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=307129&r1=307128&r2=307129&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Wed Jul 5 00:47:11 2017
@@ -1396,6 +1396,13 @@ bool Sema::hasVisibleMergedDefinition(Na
}
bool Sema::hasMergedDefinitionInCurrentModule(NamedDecl *Def) {
+ // FIXME: When not in local visibility mode, we can't tell the difference
+ // between a declaration being visible because we merged a local copy of
+ // the same declaration into it, and it being visible because its owning
+ // module is visible.
+ if (Def->getModuleOwnershipKind() == Decl::ModuleOwnershipKind::Visible &&
+ getLangOpts().ModulesLocalVisibility)
+ return true;
for (Module *Merged : Context.getModulesWithMergedDefinition(Def))
if (Merged->getTopLevelModuleName() == getLangOpts().CurrentModule)
return true;
@@ -1509,25 +1516,33 @@ bool LookupResult::isVisibleSlow(Sema &S
// FIXME: Don't assume that "same translation unit" means the same thing
// as "not from an AST file".
assert(D->isModulePrivate() && "hidden decl has no module");
- return !D->isFromASTFile();
+ if (!D->isFromASTFile() || SemaRef.hasMergedDefinitionInCurrentModule(D))
+ return true;
+ } else {
+ // If the owning module is visible, and the decl is not module private,
+ // then the decl is visible too. (Module private is ignored within the same
+ // top-level module.)
+ if (D->isModulePrivate()
+ ? DeclModule->getTopLevelModuleName() ==
+ SemaRef.getLangOpts().CurrentModule ||
+ SemaRef.hasMergedDefinitionInCurrentModule(D)
+ : SemaRef.isModuleVisible(DeclModule) ||
+ SemaRef.hasVisibleMergedDefinition(D))
+ return true;
}
- // If the owning module is visible, and the decl is not module private,
- // then the decl is visible too. (Module private is ignored within the same
- // top-level module.)
- if (D->isModulePrivate()
- ? DeclModule->getTopLevelModuleName() ==
- SemaRef.getLangOpts().CurrentModule ||
- SemaRef.hasMergedDefinitionInCurrentModule(D)
- : SemaRef.isModuleVisible(DeclModule) ||
- SemaRef.hasVisibleMergedDefinition(D))
- return true;
+ // Determine whether a decl context is a file context for the purpose of
+ // visibility. This looks through some (export and linkage spec) transparent
+ // contexts, but not others (enums).
+ auto IsEffectivelyFileContext = [](const DeclContext *DC) {
+ return DC->isFileContext() || isa<LinkageSpecDecl>(DC) ||
+ isa<ExportDecl>(DC);
+ };
- // If this declaration is not at namespace scope nor module-private,
+ // If this declaration is not at namespace scope
// then it is visible if its lexical parent has a visible definition.
DeclContext *DC = D->getLexicalDeclContext();
- if (!D->isModulePrivate() && DC && !DC->isFileContext() &&
- !isa<LinkageSpecDecl>(DC) && !isa<ExportDecl>(DC)) {
+ if (DC && !IsEffectivelyFileContext(DC)) {
// For a parameter, check whether our current template declaration's
// lexical context is visible, not whether there's some other visible
// definition of it, because parameters aren't "within" the definition.
@@ -1535,32 +1550,45 @@ bool LookupResult::isVisibleSlow(Sema &S
// In C++ we need to check for a visible definition due to ODR merging,
// and in C we must not because each declaration of a function gets its own
// set of declarations for tags in prototype scope.
- if ((D->isTemplateParameter() || isa<ParmVarDecl>(D)
- || (isa<FunctionDecl>(DC) && !SemaRef.getLangOpts().CPlusPlus))
- ? isVisible(SemaRef, cast<NamedDecl>(DC))
- : SemaRef.hasVisibleDefinition(cast<NamedDecl>(DC))) {
- if (SemaRef.CodeSynthesisContexts.empty() &&
- // FIXME: Do something better in this case.
- !SemaRef.getLangOpts().ModulesLocalVisibility) {
- // Cache the fact that this declaration is implicitly visible because
- // its parent has a visible definition.
- D->setVisibleDespiteOwningModule();
- }
- return true;
+ bool VisibleWithinParent;
+ if (D->isTemplateParameter() || isa<ParmVarDecl>(D) ||
+ (isa<FunctionDecl>(DC) && !SemaRef.getLangOpts().CPlusPlus))
+ VisibleWithinParent = isVisible(SemaRef, cast<NamedDecl>(DC));
+ else if (D->isModulePrivate()) {
+ // A module-private declaration is only visible if an enclosing lexical
+ // parent was merged with another definition in the current module.
+ VisibleWithinParent = false;
+ do {
+ if (SemaRef.hasMergedDefinitionInCurrentModule(cast<NamedDecl>(DC))) {
+ VisibleWithinParent = true;
+ break;
+ }
+ DC = DC->getLexicalParent();
+ } while (!IsEffectivelyFileContext(DC));
+ } else {
+ VisibleWithinParent = SemaRef.hasVisibleDefinition(cast<NamedDecl>(DC));
}
- return false;
+
+ if (VisibleWithinParent && SemaRef.CodeSynthesisContexts.empty() &&
+ // FIXME: Do something better in this case.
+ !SemaRef.getLangOpts().ModulesLocalVisibility) {
+ // Cache the fact that this declaration is implicitly visible because
+ // its parent has a visible definition.
+ D->setVisibleDespiteOwningModule();
+ }
+ return VisibleWithinParent;
}
+ // FIXME: All uses of DeclModule below this point should also check merged
+ // modules.
+ if (!DeclModule)
+ return false;
+
// Find the extra places where we need to look.
llvm::DenseSet<Module*> &LookupModules = SemaRef.getLookupModules();
if (LookupModules.empty())
return false;
- if (!DeclModule) {
- DeclModule = SemaRef.getOwningModule(D);
- assert(DeclModule && "hidden decl not from a module");
- }
-
// If our lookup set contains the decl's module, it's visible.
if (LookupModules.count(DeclModule))
return true;
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=307129&r1=307128&r2=307129&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Jul 5 00:47:11 2017
@@ -573,6 +573,8 @@ void ASTDeclReader::VisitDecl(Decl *D) {
else
Reader.HiddenNamesMap[Owner].push_back(D);
}
+ } else if (ModulePrivate) {
+ D->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModulePrivate);
}
}
Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=307129&r1=307128&r2=307129&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Wed Jul 5 00:47:11 2017
@@ -1462,7 +1462,7 @@ void ASTWriter::WriteControlBlock(Prepro
}
// Module map file
- if (WritingModule) {
+ if (WritingModule && WritingModule->Kind == Module::ModuleMapModule) {
Record.clear();
auto &Map = PP.getHeaderSearchInfo().getModuleMap();
Added: cfe/trunk/test/CXX/modules-ts/dcl.dcl/dcl.module/p5.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/modules-ts/dcl.dcl/dcl.module/p5.cpp?rev=307129&view=auto
==============================================================================
--- cfe/trunk/test/CXX/modules-ts/dcl.dcl/dcl.module/p5.cpp (added)
+++ cfe/trunk/test/CXX/modules-ts/dcl.dcl/dcl.module/p5.cpp Wed Jul 5 00:47:11
2017
@@ -0,0 +1,33 @@
+// RUN: rm -f %t
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -emit-module-interface %s -o %t
-DINTERFACE
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -fmodule-file=%t %s -verify
-DIMPLEMENTATION
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -fmodule-file=%t %s -verify
-DEARLY_IMPLEMENTATION
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -fmodule-file=%t %s -verify -DUSER
+
+// expected-no-diagnostics
+
+#ifdef USER
+import Foo;
+#endif
+
+#ifdef EARLY_IMPLEMENTATION
+module Foo;
+#endif
+
+template<typename T> struct type_template {
+ typedef T type;
+ void f(type);
+};
+
+template<typename T> void type_template<T>::f(type) {}
+
+template<int = 0, typename = int, template<typename> class = type_template>
+struct default_template_args {};
+
+#ifdef INTERFACE
+export module Foo;
+#endif
+
+#ifdef IMPLEMENTATION
+module Foo;
+#endif
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
