Merge pull request #31983 from hamishknight/import-regulations

lib/AST/ASTVerifier.cpp

@@ -2732,7 +2732,18 @@ public:
       PrettyStackTraceDecl debugStack("verifying GenericTypeParamDecl", GTPD);
 
       const DeclContext *DC = GTPD->getDeclContext();
-      if (!GTPD->getDeclContext()->isInnermostContextGeneric()) {
+
+      // Skip verification of deserialized generic param decls that have the
+      // the file set as their parent. This happens when they have not yet had
+      // their correct parent set.
+      // FIXME: This is a hack to workaround the fact that we don't necessarily
+      // parent a GenericTypeParamDecl if we just deserialize its type.
+      if (auto *fileDC = dyn_cast<FileUnit>(DC)) {
+        if (fileDC->getKind() == FileUnitKind::SerializedAST)
+          return;
+      }
+
+      if (!DC->isInnermostContextGeneric()) {
         Out << "DeclContext of GenericTypeParamDecl does not have "
                "generic params\n";
         abort();

lib/Frontend/Frontend.cpp

@@ -744,6 +744,10 @@ void CompilerInstance::performSemaUpTo(SourceFile::ASTStage_t LimitStage,
   if (LimitStage > SourceFile::Unprocessed &&
       Invocation.getImplicitStdlibKind() == ImplicitStdlibKind::Stdlib
       && !loadStdlib()) {
+    // If we failed to load the stdlib, mark the main module as having
+    // "failed to load", as it will contain no files.
+    // FIXME: We need to better handle a missing stdlib.
+    mainModule->setFailedToLoad();
     return;
   }
 
@@ -757,8 +761,12 @@ void CompilerInstance::performSemaUpTo(SourceFile::ASTStage_t LimitStage,
   // If we aren't in a parse-only context, load the remaining serialized inputs
   // and resolve implicit imports.
   if (LimitStage > SourceFile::Unprocessed &&
-      loadPartialModulesAndImplicitImports())
+      loadPartialModulesAndImplicitImports()) {
+    // If we failed to load a partial module, mark the main module as having
+    // "failed to load", as it may contain no files.
+    mainModule->setFailedToLoad();
     return;
+  }
 
   // Then parse all the input files.
   // FIXME: This is the only demand point for InputSourceCodeBufferIDs. We

lib/FrontendTool/FrontendTool.cpp

@@ -1209,6 +1209,20 @@ static bool emitAnyWholeModulePostTypeCheckSupplementaryOutputs(
   return hadAnyError;
 }
 
+/// Perform any actions that must have access to the ASTContext, and need to be
+/// delayed until the Swift compile pipeline has finished. This may be called
+/// before or after LLVM depending on when the ASTContext gets freed.
+static void performEndOfPipelineActions(CompilerInstance &Instance) {
+  assert(Instance.hasASTContext());
+
+  // Verify the AST for all the modules we've loaded.
+  Instance.getASTContext().verifyAllLoadedModules();
+
+  // Emit dependencies and index data.
+  emitReferenceDependenciesForAllPrimaryInputsIfNeeded(Instance);
+  emitIndexData(Instance);
+}
+
 /// Performs the compile requested by the user.
 /// \param Instance Will be reset after performIRGeneration when the verifier
 ///                 mode is NoVerify and there were no errors.
@@ -1301,15 +1315,12 @@ static bool performCompile(CompilerInstance &Instance,
   emitCompiledSourceForAllPrimaryInputsIfNeeded(Instance);
 
   SWIFT_DEFER {
-    // We might have freed the ASTContext already, but in that case we must have
+    // We might have freed the ASTContext already, but in that case we would
-    // emitted the dependencies and index first.
+    // have already performed these actions.
-    if (Instance.hasASTContext()) {
+    if (Instance.hasASTContext())
-      emitReferenceDependenciesForAllPrimaryInputsIfNeeded(Instance);
+      performEndOfPipelineActions(Instance);
-      emitIndexData(Instance);
-    }
   };
 
-
   if (Context.hadError())
     return true;
 
@@ -1504,10 +1515,9 @@ static void freeASTContextIfPossible(CompilerInstance &Instance) {
     return;
   }
 
-  // Make sure we emit dependencies and index now, because we can't do it after
+  // Make sure to perform the end of pipeline actions now, because they need
-  // the context is gone.
+  // access to the ASTContext.
-  emitReferenceDependenciesForAllPrimaryInputsIfNeeded(Instance);
+  performEndOfPipelineActions(Instance);
-  emitIndexData(Instance);
 
   Instance.freeASTContext();
 }

lib/Sema/TypeChecker.cpp

@@ -379,20 +379,6 @@ void swift::performWholeModuleTypeChecking(SourceFile &SF) {
   diagnoseObjCMethodConflicts(SF);
   diagnoseObjCUnsatisfiedOptReqConflicts(SF);
   diagnoseUnintendedObjCMethodOverrides(SF);
-
-  // In whole-module mode, import verification is deferred until all files have
-  // been type checked. This avoids caching imported declarations when a valid
-  // type checker is not present. The same declaration may need to be fully
-  // imported by subsequent files.
-  //
-  // FIXME: some playgrounds tests (playground_lvalues.swift) fail with
-  // verification enabled.
-#if 0
-  if (SF.Kind != SourceFileKind::SIL &&
-      !Ctx.LangOpts.DebuggerSupport) {
-    Ctx.verifyAllLoadedModules();
-  }
-#endif
 }
 
 bool swift::isAdditiveArithmeticConformanceDerivationEnabled(SourceFile &SF) {

lib/Serialization/Deserialization.cpp

@@ -2487,9 +2487,9 @@ public:
                                                         depth,
                                                         index);
 
-    // Always create GenericTypeParamDecls in the associated module;
+    // Always create GenericTypeParamDecls in the associated file; the real
-    // the real context will reparent them.
+    // context will reparent them.
-    auto DC = MF.getAssociatedModule();
+    auto *DC = MF.getFile();
     auto genericParam = MF.createDecl<GenericTypeParamDecl>(
         DC, MF.getIdentifier(nameID), SourceLoc(), depth, index);
     declOrOffset = genericParam;

test/Serialization/failed-partial-module.swift

@@ -0,0 +1,7 @@
+// RUN: %empty-directory(%t)
+
+// Make sure we don't crash if we fail to load a partial module.
+// RUN: %target-swift-frontend -emit-module -primary-file %s -module-name A -o %t/a.partial.swiftmodule
+// RUN: not %target-swift-frontend -emit-module -merge-modules %t/a.partial.swiftmodule -module-name Unrelated 2>&1 | %FileCheck %s
+
+// CHECK: error: cannot load module 'A' as 'Unrelated'