Merge pull request #14679 from jrose-apple/5.0-private-detective

[5.0] [ClangImporter] If a submodule named 'Private' is missing, ask Clang

rdar://problem/37553763

Author: jrose-apple

lib/ClangImporter/ClangImporter.cpp

@@ -1514,11 +1514,22 @@ ModuleDecl *ClangImporter::loadModule(
 
   // Verify that the submodule exists.
   clang::Module *submodule = clangModule;
-  for (auto component : path.slice(1)) {
+  for (auto &component : path.slice(1)) {
     submodule = submodule->findSubmodule(component.first.str());
-    // FIXME: Specialize the error for a missing submodule?
-    if (!submodule)
+
+    // Special case: a submodule named "Foo.Private" can be moved to a top-level
+    // module named "Foo_Private". Clang has special support for this.
+    // We're limiting this to just submodules named "Private" because this will
+    // put the Clang AST in a fatal error state if it /doesn't/ exist.
+    if (!submodule && component.first.str() == "Private" &&
+        (&component) == (&path[1])) {
+      submodule = loadModule(llvm::makeArrayRef(clangPath).slice(0, 2), false);
+    }
+
+    if (!submodule) {
+      // FIXME: Specialize the error for a missing submodule?
       return nullptr;
+    }
   }
 
   // Finally, load the submodule and make it visible.

lib/Sema/NameBinding.cpp

@@ -198,7 +198,13 @@ void NameBinder::addImport(
     // If we imported a submodule, import the top-level module as well.
     Identifier topLevelName = ID->getModulePath().front().first;
     topLevelModule = Context.getLoadedModule(topLevelName);
-    assert(topLevelModule && "top-level module missing");
+    if (!topLevelModule) {
+      // Clang can sometimes import top-level modules as if they were
+      // submodules.
+      assert(!M->getFiles().empty() &&
+             isa<ClangModuleUnit>(M->getFiles().front()));
+      topLevelModule = M;
+    }
   }
 
   auto *testableAttr = ID->getAttrs().getAttribute<TestableAttr>();

test/ClangImporter/Inputs/frameworks/PrivateAsParallel.framework/Headers/PrivateAsParallel.h

@@ -0,0 +1 @@
+extern int PPGlobal;

test/ClangImporter/Inputs/frameworks/PrivateAsParallel.framework/Modules/module.modulemap

@@ -0,0 +1,7 @@
+framework module PrivateAsParallel {
+  umbrella header "PrivateAsParallel.h"
+  export *
+  module * {
+    export *
+  }
+}

test/ClangImporter/Inputs/frameworks/PrivateAsParallel.framework/Modules/module.private.modulemap

@@ -0,0 +1,7 @@
+framework module PrivateAsParallel_Private {
+  umbrella header "PrivateAsParallel_Priv.h"
+  export *
+  module * {
+    export *
+  }
+}

test/ClangImporter/Inputs/frameworks/PrivateAsParallel.framework/PrivateHeaders/PrivateAsParallel_Priv.h

@@ -0,0 +1,2 @@
+#include <PrivateAsParallel/PrivateAsParallel.h>
+extern int PPPrivateGlobal;

test/ClangImporter/Inputs/frameworks/PrivateAsSubmodule.framework/Headers/PrivateAsSubmodule.h

@@ -0,0 +1 @@
+extern int PSGlobal;

test/ClangImporter/Inputs/frameworks/PrivateAsSubmodule.framework/Modules/module.modulemap

@@ -0,0 +1,7 @@
+framework module PrivateAsSubmodule {
+  umbrella header "PrivateAsSubmodule.h"
+  export *
+  module * {
+    export *
+  }
+}

test/ClangImporter/Inputs/frameworks/PrivateAsSubmodule.framework/Modules/module.private.modulemap

@@ -0,0 +1,7 @@
+explicit module PrivateAsSubmodule.Private {
+  umbrella header "PrivateAsSubmodule_Priv.h"
+  export *
+  module * {
+    export *
+  }
+}

test/ClangImporter/Inputs/frameworks/PrivateAsSubmodule.framework/PrivateHeaders/PrivateAsSubmodule_Priv.h

@@ -0,0 +1,2 @@
+#include <PrivateAsSubmodule/PrivateAsSubmodule.h>
+extern int PSPrivateGlobal;

test/ClangImporter/private_frameworks_modules.swift

@@ -0,0 +1,21 @@
+// RUN: %empty-directory(%t)
+
+// RUN: %target-swift-frontend -typecheck -F %S/Inputs/frameworks -DOLD -verify %s -Xcc -Wno-private-module
+// RUN: %target-swift-frontend -typecheck -F %S/Inputs/frameworks -DNEW -verify %s -Xcc -Wno-private-module
+
+import PrivateAsSubmodule.Private
+
+#if OLD
+import PrivateAsParallel.Private
+#elseif NEW
+import PrivateAsParallel_Private
+#else
+// This syntax wasn't supported when originally cherry-picked to this branch.
+// It's just to catch mistakes updating the test, though.
+//#error("OLD or NEW must be defined")
+#endif
+
+let _: Bool = PSGlobal // expected-error {{cannot convert value of type 'Int32'}}
+let _: Bool = PSPrivateGlobal // expected-error {{cannot convert value of type 'Int32'}}
+let _: Bool = PPGlobal // expected-error {{cannot convert value of type 'Int32'}}
+let _: Bool = PPPrivateGlobal // expected-error {{cannot convert value of type 'Int32'}}