[Clang Importer] Defer module imports to end of bridging-header parse.

There is a subtle incompatibility between the way bridging headers handle
module imports and the way clang's normal module-loading works (as done
by -emit-pch in bridging PCH, in particular).

When importing a submodule, Swift implicitly imports its supermodule. This
is part of Swift's treatment of modules and fine, we're not changing it
here.

But if client code imports a submodule, then tries to use a type that is
only defined in its supermodule, not the submodule, this _should_ cause
a parse error (and does in clang alone, or when generating a PCH).
Unfortunately Swift's "implicit parent import" currently happens
eagerly, so the supermodule is imported and its type is defined as soon
as the submodule is imported, which in turn suppresses the error.

This in turn means that client code thinks their code "works" and then
"breaks" when they turn on bridging PCH. What _should_ happen here is
that the (actually broken) client code should not be accepted in the first
place, neither bridging PCH nor textual bridging-header import.

This commit merely changes textual bridging-header import from eager
import to deferred parent-import, like bridging PCH does. This reconciles
the difference in behaviour between the two, at the cost of a source-compat
break.

rdar://30615193

Author: graydon

lib/ClangImporter/ClangImporter.cpp

@@ -83,9 +83,7 @@ namespace {
     void handleImport(const clang::Module *imported) {
       if (!imported)
         return;
-      ModuleDecl *nativeImported = Impl.finishLoadingClangModule(imported,
-        /*preferAdapter=*/true);
-      Impl.ImportedHeaderExports.push_back({ /*filter=*/{}, nativeImported });
+      Impl.DeferredHeaderImports.push_back(imported);
     }
 
     void InclusionDirective(clang::SourceLocation HashLoc,
@@ -963,6 +961,9 @@ bool ClangImporter::Implementation::importHeader(
   // Add any defined macros to the bridging header lookup table.
   addMacrosToLookupTable(*BridgingHeaderLookupTable, getNameImporter());
 
+  // Finish loading any extra modules that were (transitively) imported.
+  handleDeferredImports();
+
   // Wrap all Clang imports under a Swift import decl.
   for (auto &Import : BridgeHeaderTopLevelImports) {
     if (auto *ClangImport = Import.dyn_cast<clang::ImportDecl*>()) {
@@ -1008,16 +1009,11 @@ bool ClangImporter::importBridgingHeader(StringRef header, ModuleDecl *adapter,
                                          bool trackParsedSymbols,
                                          bool implicitImport) {
   if (llvm::sys::path::extension(header).endswith(PCH_EXTENSION)) {
-    // We already imported this with -include-pch above, so we should have
-    // collected a bunch of PCH-encoded module imports that we need to
-    // replay to the HeaderImportCallbacks for processing.
     Impl.ImportedHeaderOwners.push_back(adapter);
-    clang::ASTReader &R = *Impl.Instance->getModuleManager();
-    HeaderImportCallbacks CB(*this, Impl);
-    for (auto ID : Impl.PCHImportedSubmodules) {
-      CB.handleImport(R.getSubmodule(ID));
-    }
-    Impl.PCHImportedSubmodules.clear();
+    // We already imported this with -include-pch above, so we should have
+    // collected a bunch of PCH-encoded module imports that we just need to
+    // replay in handleDeferredImports.
+    Impl.handleDeferredImports();
     return false;
   }
   clang::FileManager &fileManager = Impl.Instance->getFileManager();
@@ -1037,7 +1033,6 @@ bool ClangImporter::importBridgingHeader(StringRef header, ModuleDecl *adapter,
     llvm::MemoryBuffer::getMemBufferCopy(
       importLine, Implementation::bridgingHeaderBufferName)
   };
-
   return Impl.importHeader(adapter, header, diagLoc, trackParsedSymbols,
                            std::move(sourceBuffer), implicitImport);
 }
@@ -1293,6 +1288,25 @@ ModuleDecl *ClangImporter::Implementation::finishLoadingClangModule(
   return result;
 }
 
+// Run through the set of deferred imports -- either those referenced by
+// submodule ID from a bridging PCH, or those already loaded as clang::Modules
+// in response to an import directive in a bridging header -- and call
+// finishLoadingClangModule on each.
+void ClangImporter::Implementation::handleDeferredImports()
+{
+  clang::ASTReader &R = *Instance->getModuleManager();
+  for (clang::serialization::SubmoduleID ID : PCHImportedSubmodules) {
+    DeferredHeaderImports.push_back(R.getSubmodule(ID));
+  }
+  PCHImportedSubmodules.clear();
+  for (const clang::Module *M : DeferredHeaderImports) {
+    ModuleDecl *nativeImported =
+      finishLoadingClangModule(M, /*preferAdapter=*/true);
+    ImportedHeaderExports.push_back({ /*filter=*/{}, nativeImported });
+  }
+  DeferredHeaderImports.clear();
+}
+
 ModuleDecl *ClangImporter::getImportedHeaderModule() const {
   return Impl.ImportedHeaderUnit->getParentModule();
 }

lib/ClangImporter/ImporterImpl.h

@@ -275,6 +275,7 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
 
   bool IsReadingBridgingPCH;
   llvm::SmallVector<clang::serialization::SubmoduleID, 2> PCHImportedSubmodules;
+  llvm::SmallVector<const clang::Module*, 2> DeferredHeaderImports;
 
   const Version CurrentVersion;
 
@@ -821,6 +822,10 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   ModuleDecl *finishLoadingClangModule(const clang::Module *clangModule,
                                        bool preferAdapter);
 
+  /// \brief Call finishLoadingClangModule on each deferred import collected
+  /// while scanning a bridging header or PCH.
+  void handleDeferredImports();
+
   /// \brief Retrieve the named Swift type, e.g., Int32.
   ///
   /// \param moduleName The name of the module in which the type should occur.