Merge pull request #37695 from egorzhdan/cxx-namespaces

C++ Interop: import namespaces redecls as separate extensions

Author: egorzhdan

lib/ClangImporter/ClangImporter.cpp

@@ -2870,6 +2870,11 @@ void ClangImporter::lookupTypeDecl(
           continue;
         }
         auto *imported = Impl.importDecl(clangDecl, Impl.CurrentVersion);
+
+        // Namespaces are imported as extensions for enums.
+        if (auto ext = dyn_cast_or_null<ExtensionDecl>(imported)) {
+          imported = ext->getExtendedNominal();
+        }
         if (auto *importedType = dyn_cast_or_null<TypeDecl>(imported)) {
           foundViaClang = true;
           receiver(importedType);
@@ -3836,11 +3841,22 @@ void ClangImporter::Implementation::lookupValue(
     // If the entry is not visible, skip it.
     if (!isVisibleClangEntry(entry)) continue;
 
-    ValueDecl *decl;
+    ValueDecl *decl = nullptr;
     // If it's a Clang declaration, try to import it.
     if (auto clangDecl = entry.dyn_cast<clang::NamedDecl *>()) {
-      decl = cast_or_null<ValueDecl>(
-          importDeclReal(clangDecl->getMostRecentDecl(), CurrentVersion));
+      bool isNamespace = isa<clang::NamespaceDecl>(clangDecl);
+      Decl *realDecl =
+          importDeclReal(clangDecl->getMostRecentDecl(), CurrentVersion,
+                         /*useCanonicalDecl*/ !isNamespace);
+
+      if (isNamespace) {
+        if (auto extension = cast_or_null<ExtensionDecl>(realDecl))
+          realDecl = extension->getExtendedNominal();
+      }
+
+      if (!realDecl)
+        continue;
+      decl = cast<ValueDecl>(realDecl);
       if (!decl) continue;
     } else if (!name.isSpecial()) {
       // Try to import a macro.

lib/ClangImporter/ImportDecl.cpp

@@ -2469,73 +2469,98 @@ namespace {
     }
 
     Decl *VisitNamespaceDecl(const clang::NamespaceDecl *decl) {
-      // If we have a name for this declaration, use it.
-      Optional<ImportedName> correctSwiftName;
-      auto importedName = importFullName(decl, correctSwiftName);
-      if (!importedName) return nullptr;
-
-      auto extensionDC =
-          Impl.importDeclContextOf(decl, importedName.getEffectiveContext());
-      if (!extensionDC)
-        return nullptr;
-
-      SourceLoc loc = Impl.importSourceLoc(decl->getBeginLoc());
       DeclContext *dc = nullptr;
       // If this is a top-level namespace, don't put it in the module we're
       // importing, put it in the "__ObjC" module that is implicitly imported.
       // This way, if we have multiple modules that all open the same namespace,
       // we won't import multiple enums with the same name in swift.
-      if (extensionDC->getContextKind() == DeclContextKind::FileUnit)
+      if (!decl->getParent()->isNamespace())
         dc = Impl.ImportedHeaderUnit;
       else {
         // This is a nested namespace, we need to find its extension decl
         // context and then use that to find the parent enum. It's important
         // that we add this to the parent enum (in the "__ObjC" module) and not
         // to the extension.
         auto parentNS = cast<clang::NamespaceDecl>(decl->getParent());
-        auto parent = Impl.importDecl(parentNS, getVersion());
+        auto parent =
+            Impl.importDecl(parentNS, getVersion(), /*UseCanonicalDecl*/ false);
         // Sometimes when the parent namespace is imported, this namespace
         // also gets imported. If that's the case, then the parent namespace
         // will be an enum (because it was able to be fully imported) in which
         // case we need to bail here.
-        auto cachedResult =
-            Impl.ImportedDecls.find({decl->getCanonicalDecl(), getVersion()});
+        auto cachedResult = Impl.ImportedDecls.find({decl, getVersion()});
         if (cachedResult != Impl.ImportedDecls.end())
           return cachedResult->second;
         dc = cast<ExtensionDecl>(parent)
                  ->getExtendedType()
                  ->getEnumOrBoundGenericEnum();
       }
-      auto *enumDecl = Impl.createDeclWithClangNode<EnumDecl>(
-          decl, AccessLevel::Public, loc,
-          importedName.getDeclName().getBaseIdentifier(),
-          Impl.importSourceLoc(decl->getLocation()), None, nullptr, dc);
-      if (isa<clang::NamespaceDecl>(decl->getParent()))
-        cast<EnumDecl>(dc)->addMember(enumDecl);
-
-      // We are creating an extension, so put it at the top level. This is done
-      // after creating the enum, though, because we may need the correctly
-      // nested decl context above when creating the enum.
-      while (extensionDC->getParent() &&
-             extensionDC->getContextKind() != DeclContextKind::FileUnit)
-        extensionDC = extensionDC->getParent();
-
-      auto *extension = ExtensionDecl::create(Impl.SwiftContext, loc, nullptr,
-                                              {}, extensionDC, nullptr, decl);
-      Impl.SwiftContext.evaluator.cacheOutput(ExtendedTypeRequest{extension},
-                                              enumDecl->getDeclaredType());
-      Impl.SwiftContext.evaluator.cacheOutput(ExtendedNominalRequest{extension},
-                                              std::move(enumDecl));
-      // Keep track of what members we've already added so we don't add the same
-      // member twice. Note: we can't use "ImportedDecls" for this because we
-      // might import a decl that we don't add (for example, if it was a
-      // parameter to another decl).
-      SmallPtrSet<Decl *, 16> addedMembers;
+
+      EnumDecl *enumDecl = nullptr;
+      // Try to find an already created enum for this namespace.
       for (auto redecl : decl->redecls()) {
-        // This will be reset as the EnumDecl after we return from
-        // VisitNamespaceDecl.
-        Impl.ImportedDecls[{redecl->getCanonicalDecl(), getVersion()}] =
-            extension;
+        auto extension = Impl.ImportedDecls.find({redecl, getVersion()});
+        if (extension != Impl.ImportedDecls.end()) {
+          enumDecl = cast<EnumDecl>(
+              cast<ExtensionDecl>(extension->second)->getExtendedNominal());
+          break;
+        }
+      }
+      // If we're seeing this namespace for the first time, we need to create a
+      // new enum in the "__ObjC" module.
+      if (!enumDecl) {
+        // If we don't have a name for this declaration, bail.
+        Optional<ImportedName> correctSwiftName;
+        auto importedName = importFullName(decl, correctSwiftName);
+        if (!importedName)
+          return nullptr;
+
+        enumDecl = Impl.createDeclWithClangNode<EnumDecl>(
+            decl, AccessLevel::Public,
+            Impl.importSourceLoc(decl->getBeginLoc()),
+            importedName.getDeclName().getBaseIdentifier(),
+            Impl.importSourceLoc(decl->getLocation()), None, nullptr, dc);
+        if (isa<clang::NamespaceDecl>(decl->getParent()))
+          cast<EnumDecl>(dc)->addMember(enumDecl);
+      }
+
+      for (auto redecl : decl->redecls()) {
+        if (Impl.ImportedDecls.find({redecl, getVersion()}) !=
+            Impl.ImportedDecls.end())
+          continue;
+
+        Optional<ImportedName> correctSwiftName;
+        auto importedName = importFullName(redecl, correctSwiftName);
+        if (!importedName)
+          continue;
+
+        auto extensionDC = Impl.importDeclContextOf(
+            redecl, importedName.getEffectiveContext());
+        if (!extensionDC)
+          continue;
+
+        // We are creating an extension, so put it at the top level. This is
+        // done after creating the enum, though, because we may need the
+        // correctly nested decl context above when creating the enum.
+        while (extensionDC->getParent() &&
+               extensionDC->getContextKind() != DeclContextKind::FileUnit)
+          extensionDC = extensionDC->getParent();
+
+        auto *extension = ExtensionDecl::create(
+            Impl.SwiftContext, Impl.importSourceLoc(decl->getBeginLoc()),
+            nullptr, {}, extensionDC, nullptr, redecl);
+        enumDecl->addExtension(extension);
+        Impl.ImportedDecls[{redecl, getVersion()}] = extension;
+
+        Impl.SwiftContext.evaluator.cacheOutput(ExtendedTypeRequest{extension},
+                                                enumDecl->getDeclaredType());
+        Impl.SwiftContext.evaluator.cacheOutput(ExtendedNominalRequest{extension},
+                                                std::move(enumDecl));
+        // Keep track of what members we've already added so we don't add the
+        // same member twice. Note: we can't use "ImportedDecls" for this
+        // because we might import a decl that we don't add (for example, if it
+        // was a parameter to another decl).
+        SmallPtrSet<Decl *, 16> addedMembers;
 
         // Insert these backwards into "namespaceDecls" so we can pop them off
         // the end without loosing order.
@@ -2566,7 +2591,8 @@ namespace {
             continue;
           }
 
-          auto member = Impl.importDecl(nd, getVersion());
+          bool useCanonicalDecl = !isa<clang::NamespaceDecl>(nd);
+          auto member = Impl.importDecl(nd, getVersion(), useCanonicalDecl);
           if (!member || addedMembers.count(member) ||
               isa<clang::NamespaceDecl>(nd))
             continue;
@@ -2581,10 +2607,7 @@ namespace {
         }
       }
 
-      if (!extension->getMembers().empty())
-        enumDecl->addExtension(extension);
-
-      return enumDecl;
+      return Impl.ImportedDecls[{decl, getVersion()}];
     }
 
     Decl *VisitUsingDirectiveDecl(const clang::UsingDirectiveDecl *decl) {
@@ -9235,7 +9258,8 @@ DeclContext *ClangImporter::Implementation::importDeclContextImpl(
   // Category decls with same name can be merged and using canonical decl always
   // leads to the first category of the given name. We'd like to keep these
   // categories separated.
-  auto useCanonical = !isa<clang::ObjCCategoryDecl>(decl);
+  auto useCanonical =
+      !isa<clang::ObjCCategoryDecl>(decl) && !isa<clang::NamespaceDecl>(decl);
   auto swiftDecl = importDeclForDeclContext(ImportingDecl, decl->getName(),
                                             decl, CurrentVersion, useCanonical);
   if (!swiftDecl)

lib/ClangImporter/ImporterImpl.h

@@ -881,10 +881,11 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   ///
   /// \returns The imported declaration, or null if this declaration could
   /// not be represented in Swift.
-  Decl *importDeclReal(const clang::NamedDecl *ClangDecl, Version version) {
+  Decl *importDeclReal(const clang::NamedDecl *ClangDecl, Version version,
+                       bool useCanonicalDecl = true) {
     return importDeclAndCacheImpl(ClangDecl, version,
                                   /*SuperfluousTypedefsAreTransparent=*/false,
-                                  /*UseCanonicalDecl*/true);
+                                  /*UseCanonicalDecl*/ useCanonicalDecl);
   }
 
   /// Import a cloned version of the given declaration, which is part of

lib/ClangImporter/SwiftLookupTable.h

@@ -197,11 +197,10 @@ class EffectiveClangContext {
       DC = omDecl->getCanonicalDecl();
     } else if (auto fDecl = dyn_cast<clang::FunctionDecl>(dc)) {
       DC = fDecl->getCanonicalDecl();
-    } else if (auto nsDecl = dyn_cast<clang::NamespaceDecl>(dc)) {
-      DC = nsDecl->getCanonicalDecl();
     } else {
       assert(isa<clang::TranslationUnitDecl>(dc) ||
              isa<clang::LinkageSpecDecl>(dc) ||
+             isa<clang::NamespaceDecl>(dc) ||
              isa<clang::ObjCContainerDecl>(dc) &&
                  "No other kinds of effective Clang contexts");
       DC = dc;

test/Interop/Cxx/namespace/Inputs/module.modulemap

@@ -22,6 +22,17 @@ module FreeFunctionsSecondHeader {
   requires cplusplus
 }
 
+module Submodules {
+  module SubmoduleA {
+    header "submodule-a.h"
+    requires cplusplus
+  }
+  module SubmoduleB {
+    header "submodule-b.h"
+    requires cplusplus
+  }
+}
+
 module Templates {
   header "templates.h"
   requires cplusplus

test/Interop/Cxx/namespace/Inputs/submodule-a.h

@@ -0,0 +1,16 @@
+#ifndef TEST_INTEROP_CXX_NAMESPACE_INPUTS_SUBMODULE_A_H
+#define TEST_INTEROP_CXX_NAMESPACE_INPUTS_SUBMODULE_A_H
+
+namespace NS1 {
+
+namespace NS2 {
+
+struct BasicDeepA {};
+
+} // namespace NS2
+
+struct BasicA {};
+
+} // namespace NS1
+
+#endif // TEST_INTEROP_CXX_NAMESPACE_INPUTS_SUBMODULE_A_H

test/Interop/Cxx/namespace/Inputs/submodule-b.h

@@ -0,0 +1,16 @@
+#ifndef TEST_INTEROP_CXX_NAMESPACE_INPUTS_SUBMODULE_B_H
+#define TEST_INTEROP_CXX_NAMESPACE_INPUTS_SUBMODULE_B_H
+
+namespace NS1 {
+
+namespace NS2 {
+
+struct BasicDeepB {};
+
+} // namespace NS2
+
+struct BasicB {};
+
+} // namespace NS1
+
+#endif // TEST_INTEROP_CXX_NAMESPACE_INPUTS_SUBMODULE_B_H

test/Interop/Cxx/namespace/classes-module-interface.swift

@@ -1,7 +1,7 @@
 // RUN: %target-swift-ide-test -print-module -module-to-print=Classes -I %S/Inputs -source-filename=x -enable-cxx-interop | %FileCheck %s
 
 // CHECK-NOT: extension
-// CHECK: extension ClassesNS1.ClassesNS2 {
+// CHECK: extension ClassesNS1 {
 // CHECK:   struct BasicStruct {
 // CHECK:     init()
 // CHECK:     mutating func basicMember() -> UnsafePointer<CChar>!
@@ -13,7 +13,7 @@
 // CHECK: }
 // CHECK-NOT: extension
 
-// CHECK: extension ClassesNS1 {
+// CHECK: extension ClassesNS1.ClassesNS2 {
 // CHECK:   struct BasicStruct {
 // CHECK:     init()
 // CHECK:     mutating func basicMember() -> UnsafePointer<CChar>!

test/Interop/Cxx/namespace/free-functions-module-interface.swift

@@ -1,24 +1,36 @@
 // RUN: %target-swift-ide-test -print-module -module-to-print=FreeFunctions -I %S/Inputs -source-filename=x -enable-cxx-interop | %FileCheck %s
 
 // CHECK-NOT: extension
+// CHECK: extension FunctionsNS1 {
+// CHECK:   static func basicFunctionTopLevel() -> UnsafePointer<CChar>!
+// CHECK:   static func definedOutOfLine() -> UnsafePointer<CChar>!
+// CHECK:   static func forwardDeclared() -> UnsafePointer<CChar>!
+// CHECK: }
+// CHECK-NOT: extension
+
+// CHECK: extension FunctionsNS1.FunctionsNS2 {
+// CHECK:   static func basicFunctionSecondLevel() -> UnsafePointer<CChar>!
+// CHECK: }
+// CHECK-NOT: extension
+
 // CHECK: extension FunctionsNS1.FunctionsNS2.FunctionsNS3 {
 // CHECK:   static func basicFunctionLowestLevel() -> UnsafePointer<CChar>!
 // CHECK: }
 // CHECK-NOT: extension
 
+// CHECK: extension FunctionsNS1 {
+// CHECK:   static func definedInDefs() -> UnsafePointer<CChar>!
+// CHECK: }
+// CHECK-NOT: extension
+
 // CHECK: extension FunctionsNS1 {
 // CHECK:   static func sameNameInChild() -> UnsafePointer<CChar>!
 // CHECK:   static func sameNameInSibling() -> UnsafePointer<CChar>!
-// CHECK:   static func definedInDefs() -> UnsafePointer<CChar>!
-// CHECK:   static func forwardDeclared() -> UnsafePointer<CChar>!
-// CHECK:   static func basicFunctionTopLevel() -> UnsafePointer<CChar>!
-// CHECK:   static func definedOutOfLine() -> UnsafePointer<CChar>!
 // CHECK: }
 // CHECK-NOT: extension
 
 // CHECK: extension FunctionsNS1.FunctionsNS2 {
 // CHECK:   static func sameNameInChild() -> UnsafePointer<CChar>!
-// CHECK:   static func basicFunctionSecondLevel() -> UnsafePointer<CChar>!
 // CHECK: }
 // CHECK-NOT: extension
 

test/Interop/Cxx/namespace/free-functions-second-header-module-interface.swift

@@ -2,6 +2,5 @@
 
 // TODO: This file doesn't really test anything because functions need not be defined.
 // CHECK: extension FunctionsNS1 {
-// CHECK-NOT: extension
 // CHECK:   static func definedInDefs() -> UnsafePointer<CChar>!
 // CHECK: }