[modules ts] Declarations from a module interface unit are only visible outside

the module if declared in an export block. 


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@307115 91177308-0d34-0410-b5e6-96231b3b80d8

12 files changed, 134 insertions, 23 deletions

diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h
index 0f1f481ae4..8658d886a9 100644
--- a/include/clang/AST/DeclBase.h
+++ b/include/clang/AST/DeclBase.h
@@ -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 (hasOwningModule())
+    if (getModuleOwnershipKind() == ModuleOwnershipKind::VisibleWhenImported)
       setModuleOwnershipKind(ModuleOwnershipKind::Visible);
   }
 
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 1fc0b2e502..da961ab7e8 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -1266,6 +1266,7 @@ public:
 
   void emitAndClearUnusedLocalTypedefWarnings();
 
+  void ActOnStartOfTranslationUnit();
   void ActOnEndOfTranslationUnit();
 
   void CheckDelegatingCtorCycles();
@@ -1541,6 +1542,7 @@ public:
                                  llvm::SmallVectorImpl<Module *> *Modules);
 
   bool hasVisibleMergedDefinition(NamedDecl *Def);
+  bool hasMergedDefinitionInCurrentModule(NamedDecl *Def);
 
   /// Determine if \p D and \p Suggested have a structurally compatible
   /// layout as described in C11 6.2.7/1.
diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp
index a0594a0203..842ae9b6cc 100644
--- a/lib/AST/DeclBase.cpp
+++ b/lib/AST/DeclBase.cpp
@@ -283,7 +283,7 @@ void Decl::setLexicalDeclContext(DeclContext *DC) {
       setLocalOwningModule(cast<Decl>(DC)->getOwningModule());
   }
 
-  assert((!hasOwningModule() || getOwningModule()) &&
+  assert((!hasOwningModule() || getOwningModule() || isModulePrivate()) &&
          "hidden declaration has no owning module");
 }
 
diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp
index af29b5e9c6..1ed7ef9663 100644
--- a/lib/Parse/Parser.cpp
+++ b/lib/Parse/Parser.cpp
@@ -526,6 +526,8 @@ void Parser::LateTemplateParserCleanupCallback(void *P) {
 }
 
 bool Parser::ParseFirstTopLevelDecl(DeclGroupPtrTy &Result) {
+  Actions.ActOnStartOfTranslationUnit();
+
   // C11 6.9p1 says translation units must have at least one top-level
   // declaration. C++ doesn't have this restriction. We also don't want to
   // complain if we have a precompiled header, although technically if the PCH
diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index 34f5e26be8..dc9f977d41 100644
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -705,6 +705,18 @@ void Sema::emitAndClearUnusedLocalTypedefWarnings() {
   UnusedLocalTypedefNameCandidates.clear();
 }
 
+/// This is called before the very first declaration in the translation unit
+/// is parsed. Note that the ASTContext may have already injected some
+/// declarations.
+void Sema::ActOnStartOfTranslationUnit() {
+  if (getLangOpts().ModulesTS) {
+    // We start in the global module; all those declarations are implicitly
+    // module-private (though they do not have module linkage).
+    Context.getTranslationUnitDecl()->setModuleOwnershipKind(
+        Decl::ModuleOwnershipKind::ModulePrivate);
+  }
+}
+
 /// ActOnEndOfTranslationUnit - This is called at the very end of the
 /// translation unit when EOF is reached and all but the top-level scope is
 /// popped.
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index ab2661cd71..31b24f91c1 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -16054,8 +16054,6 @@ Sema::DeclGroupPtrTy Sema::ActOnModuleDecl(SourceLocation StartLoc,
     return nullptr;
   }
 
-  // FIXME: Create a ModuleDecl and return it.
-
   // FIXME: Most of this work should be done by the preprocessor rather than
   // here, in order to support macro import.
 
@@ -16069,6 +16067,8 @@ Sema::DeclGroupPtrTy Sema::ActOnModuleDecl(SourceLocation StartLoc,
     ModuleName += Piece.first->getName();
   }
 
+  // FIXME: If we've already seen a module-declaration, report an error.
+
   // If a module name was explicitly specified on the command line, it must be
   // correct.
   if (!getLangOpts().CurrentModule.empty() &&
@@ -16081,6 +16081,7 @@ Sema::DeclGroupPtrTy Sema::ActOnModuleDecl(SourceLocation StartLoc,
   const_cast<LangOptions&>(getLangOpts()).CurrentModule = ModuleName;
 
   auto &Map = PP.getHeaderSearchInfo().getModuleMap();
+  Module *Mod;
 
   switch (MDK) {
   case ModuleDeclKind::Module: {
@@ -16099,12 +16100,9 @@ Sema::DeclGroupPtrTy Sema::ActOnModuleDecl(SourceLocation StartLoc,
     }
 
     // Create a Module for the module that we're defining.
-    Module *Mod = Map.createModuleForInterfaceUnit(ModuleLoc, ModuleName);
+    Mod = Map.createModuleForInterfaceUnit(ModuleLoc, ModuleName);
     assert(Mod && "module creation should not fail");
-
-    // Enter the semantic scope of the module.
-    ActOnModuleBegin(ModuleLoc, Mod);
-    return nullptr;
+    break;
   }
 
   case ModuleDeclKind::Partition:
@@ -16114,14 +16112,26 @@ Sema::DeclGroupPtrTy Sema::ActOnModuleDecl(SourceLocation StartLoc,
   case ModuleDeclKind::Implementation:
     std::pair<IdentifierInfo *, SourceLocation> ModuleNameLoc(
         PP.getIdentifierInfo(ModuleName), Path[0].second);
-
-    DeclResult Import = ActOnModuleImport(ModuleLoc, ModuleLoc, ModuleNameLoc);
-    if (Import.isInvalid())
+    Mod = getModuleLoader().loadModule(ModuleLoc, Path, Module::AllVisible,
+                                       /*IsIncludeDirective=*/false);
+    if (!Mod)
       return nullptr;
-    return ConvertDeclToDeclGroup(Import.get());
+    break;
   }
 
-  llvm_unreachable("unexpected module decl kind");
+  // Enter the semantic scope of the module.
+  ModuleScopes.push_back({});
+  ModuleScopes.back().Module = Mod;
+  ModuleScopes.back().OuterVisibleModules = std::move(VisibleModules);
+  VisibleModules.setVisible(Mod, ModuleLoc);
+
+  // From now on, we have an owning module for all declarations we see.
+  // However, those declarations are module-private unless explicitly
+  // exported.
+  Context.getTranslationUnitDecl()->setLocalOwningModule(Mod);
+
+  // FIXME: Create a ModuleDecl.
+  return nullptr;
 }
 
 DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
@@ -16310,6 +16320,7 @@ Decl *Sema::ActOnStartExportDecl(Scope *S, SourceLocation ExportLoc,
 
   CurContext->addDecl(D);
   PushDeclContext(S, D);
+  D->setModuleOwnershipKind(Decl::ModuleOwnershipKind::VisibleWhenImported);
   return D;
 }
 
diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp
index 2e7fb875a2..63b3f6a1cb 100644
--- a/lib/Sema/SemaLookup.cpp
+++ b/lib/Sema/SemaLookup.cpp
@@ -1395,6 +1395,13 @@ bool Sema::hasVisibleMergedDefinition(NamedDecl *Def) {
   return false;
 }
 
+bool Sema::hasMergedDefinitionInCurrentModule(NamedDecl *Def) {
+  for (Module *Merged : Context.getModulesWithMergedDefinition(Def))
+    if (Merged->getTopLevelModuleName() == getLangOpts().CurrentModule)
+      return true;
+  return false;
+}
+
 template<typename ParmDecl>
 static bool
 hasVisibleDefaultArgument(Sema &S, const ParmDecl *D,
@@ -1495,16 +1502,25 @@ bool LookupResult::isVisibleSlow(Sema &SemaRef, NamedDecl *D) {
   assert(D->isHidden() && "should not call this: not in slow case");
 
   Module *DeclModule = SemaRef.getOwningModule(D);
-  assert(DeclModule && "hidden decl not from a module");
+  if (!DeclModule) {
+    // A module-private declaration with no owning module means this is in the
+    // global module in the C++ Modules TS. This is visible within the same
+    // translation unit only.
+    // 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 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.)
-  // FIXME: Check the owning module for module-private declarations rather than
-  // assuming "same AST file" is the same thing as "same module".
-  if ((!D->isFromASTFile() || !D->isModulePrivate()) &&
-      (SemaRef.isModuleVisible(DeclModule) ||
-       SemaRef.hasVisibleMergedDefinition(D)))
+  if (D->isModulePrivate()
+        ? DeclModule->getTopLevelModuleName() ==
+                  SemaRef.getLangOpts().CurrentModule ||
+          SemaRef.hasMergedDefinitionInCurrentModule(D)
+        : SemaRef.isModuleVisible(DeclModule) ||
+          SemaRef.hasVisibleMergedDefinition(D))
     return true;
 
   // If this declaration is not at namespace scope nor module-private,
diff --git a/test/CXX/modules-ts/basic/basic.link/p2/module.cpp b/test/CXX/modules-ts/basic/basic.link/p2/module.cpp
new file mode 100644
index 0000000000..3fc6044d8e
--- /dev/null
+++ b/test/CXX/modules-ts/basic/basic.link/p2/module.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts %S/module.cppm -emit-module-interface -o %t
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -fmodule-file=%t %s -verify
+// expected-no-diagnostics
+module M;
+
+// FIXME: Use of internal linkage entities should be rejected.
+void use_from_module_impl() {
+  external_linkage_fn();
+  module_linkage_fn();
+  internal_linkage_fn();
+  (void)external_linkage_class{};
+  (void)module_linkage_class{};
+  (void)internal_linkage_class{};
+  (void)external_linkage_var;
+  (void)module_linkage_var;
+  (void)internal_linkage_var;
+}
diff --git a/test/CXX/modules-ts/basic/basic.link/p2/module.cppm b/test/CXX/modules-ts/basic/basic.link/p2/module.cppm
new file mode 100644
index 0000000000..bb261700db
--- /dev/null
+++ b/test/CXX/modules-ts/basic/basic.link/p2/module.cppm
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts %s -verify
+// expected-no-diagnostics
+export module M;
+
+export int external_linkage_var;
+int module_linkage_var;
+static int internal_linkage_var;
+
+export void external_linkage_fn() {}
+void module_linkage_fn() {}
+static void internal_linkage_fn() {}
+
+export struct external_linkage_class {};
+struct module_linkage_class {};
+namespace {
+  struct internal_linkage_class {};
+}
+
+void use() {
+  external_linkage_fn();
+  module_linkage_fn();
+  internal_linkage_fn();
+  (void)external_linkage_class{};
+  (void)module_linkage_class{};
+  (void)internal_linkage_class{};
+  (void)external_linkage_var;
+  (void)module_linkage_var;
+  (void)internal_linkage_var;
+}
diff --git a/test/CXX/modules-ts/basic/basic.link/p2/other.cpp b/test/CXX/modules-ts/basic/basic.link/p2/other.cpp
new file mode 100644
index 0000000000..8370777e7e
--- /dev/null
+++ b/test/CXX/modules-ts/basic/basic.link/p2/other.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts %S/module.cppm -emit-module-interface -o %t
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -fmodule-file=%t %s -verify
+import M;
+
+void use_from_module_impl() {
+  external_linkage_fn();
+  module_linkage_fn(); // expected-error {{undeclared identifier}}
+  internal_linkage_fn(); // expected-error {{undeclared identifier}}
+  (void)external_linkage_class{};
+  (void)module_linkage_class{}; // expected-error {{undeclared identifier}} expected-error 0+{{}}
+  (void)internal_linkage_class{}; // expected-error {{undeclared identifier}} expected-error 0+{{}}
+  // expected-note@module.cppm:9 {{here}}
+  (void)external_linkage_var;
+  (void)module_linkage_var; // expected-error {{undeclared identifier}}
+  (void)internal_linkage_var; // expected-error {{undeclared identifier}}
+}
diff --git a/test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.import/p1.cpp b/test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.import/p1.cpp
index aaf43d6584..aad31b4b46 100644
--- a/test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.import/p1.cpp
+++ b/test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.import/p1.cpp
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: mkdir -p %t
-// RUN: echo 'export module x; int a, b;' > %t/x.cppm
-// RUN: echo 'export module x.y; int c;' > %t/x.y.cppm
+// RUN: echo 'export module x; export int a, b;' > %t/x.cppm
+// RUN: echo 'export module x.y; export int c;' > %t/x.y.cppm
 //
 // RUN: %clang_cc1 -std=c++1z -fmodules-ts -emit-module-interface %t/x.cppm -o %t/x.pcm
 // RUN: %clang_cc1 -std=c++1z -fmodules-ts -emit-module-interface -fmodule-file=%t/x.pcm %t/x.y.cppm -o %t/x.y.pcm
diff --git a/test/SemaCXX/modules-ts.cppm b/test/SemaCXX/modules-ts.cppm
index 29122ec7da..2ea4958bff 100644
--- a/test/SemaCXX/modules-ts.cppm
+++ b/test/SemaCXX/modules-ts.cppm
@@ -13,7 +13,13 @@ export module foo;
 // expected-note@modules-ts.cppm:* {{loaded from}}
 #endif
 
-static int m; // ok, internal linkage, so no redefinition error
+static int m;
+#if TEST == 2 // FIXME: 'm' has internal linkage, so there should be no error here
+// expected-error@-2 {{redefinition of '}}
+// expected-note@-3 {{unguarded header; consider using #ifdef guards or #pragma once}}
+// FIXME: We should drop the "header from" in this diagnostic.
+// expected-note-re@modules-ts.cppm:1 {{'{{.*}}modules-ts.cppm' included multiple times, additional include site in header from module 'foo'}}
+#endif
 int n;
 #if TEST >= 2
 // expected-error@-2 {{redefinition of '}}