[Modules TS] Fix namespace visibility

[hamzasood]

Summary

Namespaces without an explicit export were being mistakenly marked as module private.
This patch fixes the visibility as per [basic.namespace]p1 and adds a test case with previously rejected (yet valid) code.

[rsmith]

lib/AST/DeclBase.cpp

285

"because they're" -> "because they might be" -- this only applies to Modules TS modules, not to header modules.

lib/Sema/SemaDeclCXX.cpp

8629

This should depend on the kind of module we're in, not whether -fmodules-ts is enabled.

[hamzasood]

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

include/clang/AST/DeclBase.h

protected:
   void *operator new(std::size_t Size, const ASTContext &Ctx,
                      DeclContext *Parent, std::size_t Extra = 0);
 
 private:
   bool AccessDeclContextSanity() const;
 
   /// 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)
       : NextInContextAndBits(nullptr, getModuleOwnershipKindForChildOf(DC)),
         DeclCtx(DC), Loc(L), DeclKind(DK), InvalidDecl(false), HasAttrs(false),
         Implicit(false), Used(false), Referenced(false),
         TopLevelDeclInObjCContainer(false), Access(AS_none), FromASTFile(0),
         IdentifierNamespace(getIdentifierNamespaceForKind(DK)),

lib/AST/DeclBase.cpp

 
 //===----------------------------------------------------------------------===//
 // Decl Implementation
 //===----------------------------------------------------------------------===//
 
 // Out-of-line virtual method providing a home for Decl.
 Decl::~Decl() = default;
 
+Decl::ModuleOwnershipKind Decl::getModuleOwnershipKindForChildOf(DeclContext *DC) {
+  // Ignore namespaces because they're visible by default.
+  while (DC && DC->isNamespace())
+    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;
 }
 
 void Decl::setLexicalDeclContext(DeclContext *DC) {
   if (DC == getLexicalDeclContext())
     return;
 

lib/Sema/SemaDeclCXX.cpp

Decl *Sema::ActOnStartNamespaceDef(Scope *NamespcScope,
   ActOnDocumentableDecl(Namespc);
 
   // Although we could have an invalid decl (i.e. the namespace name is a
   // redefinition), push it as current DeclContext and try to continue parsing.
   // FIXME: We should be able to push Namespc here, so that the each DeclContext
   // for the namespace has the declarations that showed up in that particular
   // namespace definition.
   PushDeclContext(NamespcScope, Namespc);
+
+  // A namespace with external linkage should always be visible when imported,
+  // regardless of whether or not it's explicitly exported.
+  if (getLangOpts().ModulesTS &&
+      !Namespc->isAnonymousNamespace() && !Namespc->isInAnonymousNamespace()) {
+    const Module *CurrentModule = getCurrentModule();
+    assert(CurrentModule);
+    Namespc->setModuleOwnershipKind(
+        CurrentModule->Kind == Module::ModuleKind::GlobalModuleFragment
+            ? Decl::ModuleOwnershipKind::Visible
+            : Decl::ModuleOwnershipKind::VisibleWhenImported);
+  }
+
   return Namespc;
 }
 
 /// getNamespaceDecl - Returns the namespace a decl represents. If the decl
 /// is a namespace alias, returns the namespace it points to.
 static inline NamespaceDecl *getNamespaceDecl(NamedDecl *D) {
   if (NamespaceAliasDecl *AD = dyn_cast_or_null<NamespaceAliasDecl>(D))
     return AD->getNamespace();

test/CXX/modules-ts/basic/basic.namespace/p1.cpp

+// 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