[PATCH] D41627: [Modules TS] Fix namespace visibility

[Hamza Sood via Phabricator via cfe-commits]

hamzasood updated this revision to Diff 156689.
hamzasood added a comment.

- Don't assume that -fmodules-ts implies that we're in a TS module.
- Don't ignore private namespaces when calculating the ownership kind.


https://reviews.llvm.org/D41627

Files:
  include/clang/AST/DeclBase.h
  lib/AST/DeclBase.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/CXX/modules-ts/basic/basic.namespace/p1.cpp


Index: test/CXX/modules-ts/basic/basic.namespace/p1.cpp
===================================================================
--- test/CXX/modules-ts/basic/basic.namespace/p1.cpp
+++ test/CXX/modules-ts/basic/basic.namespace/p1.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fmodules-ts -DMODULE -emit-module-interface %s -o %t
+// RUN: %clang_cc1 -fmodules-ts -DUSER -fmodule-file=%t %s -verify
+// expected-no-diagnostics
+
+#ifdef MODULE
+export module A;
+namespace ns {
+  export class A { };
+}
+#endif
+
+#ifdef USER
+import A;
+ns::A a;
+#endif
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -8866,6 +8866,21 @@
   // for the namespace has the declarations that showed up in that particular
   // namespace definition.
   PushDeclContext(NamespcScope, Namespc);
+
+  if (const Module *CurrentModule = getCurrentModule()) {
+    // Under the Modules TS, a namespace with external linkage should always be
+    // visible when imported, regardless of whether it's explicitly exported.
+    const bool IsTSModule = CurrentModule->Kind == Module::ModuleInterfaceUnit ||
+                            CurrentModule->Kind == Module::GlobalModuleFragment;
+    if (IsTSModule &&
+        !Namespc->isAnonymousNamespace() && !Namespc->isInAnonymousNamespace()) {
+      Namespc->setModuleOwnershipKind(
+        CurrentModule->Kind == Module::ModuleKind::GlobalModuleFragment
+            ? Decl::ModuleOwnershipKind::Visible
+            : Decl::ModuleOwnershipKind::VisibleWhenImported);
+    }
+  }
+
   return Namespc;
 }
 
Index: lib/AST/DeclBase.cpp
===================================================================
--- lib/AST/DeclBase.cpp
+++ lib/AST/DeclBase.cpp
@@ -292,6 +292,23 @@
 // Out-of-line virtual method providing a home for Decl.
 Decl::~Decl() = default;
 
+Decl::ModuleOwnershipKind Decl::getModuleOwnershipKindForChildOf(DeclContext *DC) {
+  // Ignore public namespaces because they might be visible by default.
+  while (DC && DC->isNamespace() && !cast<Decl>(DC)->isModulePrivate())
+    DC = DC->getParent();
+
+  if (auto *D = cast_or_null<Decl>(DC)) {
+    auto MOK = D->getModuleOwnershipKind();
+    if (MOK != ModuleOwnershipKind::Unowned &&
+        (!D->isFromASTFile() || D->hasLocalOwningModuleStorage()))
+      return MOK;
+    // If D is not local and we have no local module storage, then we don't
+    // need to track module ownership at all.
+  }
+
+  return ModuleOwnershipKind::Unowned;
+}
+
 void Decl::setDeclContext(DeclContext *DC) {
   DeclCtx = DC;
 }
Index: include/clang/AST/DeclBase.h
===================================================================
--- include/clang/AST/DeclBase.h
+++ include/clang/AST/DeclBase.h
@@ -350,18 +350,7 @@
 
   /// Get the module ownership kind to use for a local lexical child of \p DC,
   /// which may be either a local or (rarely) an imported declaration.
-  static ModuleOwnershipKind getModuleOwnershipKindForChildOf(DeclContext *DC) {
-    if (DC) {
-      auto *D = cast<Decl>(DC);
-      auto MOK = D->getModuleOwnershipKind();
-      if (MOK != ModuleOwnershipKind::Unowned &&
-          (!D->isFromASTFile() || D->hasLocalOwningModuleStorage()))
-        return MOK;
-      // If D is not local and we have no local module storage, then we don't
-      // need to track module ownership at all.
-    }
-    return ModuleOwnershipKind::Unowned;
-  }
+  static ModuleOwnershipKind getModuleOwnershipKindForChildOf(DeclContext *DC);
 
 protected:
   Decl(Kind DK, DeclContext *DC, SourceLocation L)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41627.156689.patch
Type: text/x-patch
Size: 3604 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180721/57709790/attachment.bin>