Make sure all failures to load module interfaces are diagnosed (#25636)

...specifically, diagnosed in the parent DiagnosticEngine. This not
only provides a better user experience, but makes sure that the
compiler exits with a nonzero exit code even if the module goes
unused.

rdar://problem/50789839

Author: jrose-apple

include/swift/AST/DiagnosticsSema.def

@@ -628,7 +628,7 @@ ERROR(sema_opening_import,Fatal,
       "opening import file for module %0: %1", (Identifier, StringRef))
 
 ERROR(serialization_load_failed,Fatal,
-      "failed to load module %0", (Identifier))
+      "failed to load module '%0'", (StringRef))
 ERROR(serialization_malformed_module,Fatal,
       "malformed compiled module: %0", (StringRef))
 ERROR(serialization_module_too_new,Fatal,

lib/Frontend/ParseableInterfaceModuleLoader.cpp

@@ -18,6 +18,7 @@
 #include "swift/AST/FileSystem.h"
 #include "swift/AST/Module.h"
 #include "swift/AST/ProtocolConformance.h"
+#include "swift/Basic/Defer.h"
 #include "swift/Basic/Lazy.h"
 #include "swift/Basic/Platform.h"
 #include "swift/Basic/STLExtras.h"
@@ -564,6 +565,17 @@ class swift::ParseableInterfaceBuilder {
 
       SubInstance.createDependencyTracker(FEOpts.TrackSystemDeps);
 
+      SWIFT_DEFER {
+        // Make sure to emit a generic top-level error if a module fails to
+        // load. This is not only good for users; it also makes sure that we've
+        // emitted an error in the parent diagnostic engine, which is what
+        // determines whether the process exits with a proper failure status.
+        if (SubInstance.getASTContext().hadError()) {
+          diags.diagnose(diagnosticLoc, diag::serialization_load_failed,
+                         moduleName);
+        }
+      };
+
       if (SubInstance.setup(subInvocation)) {
         SubError = true;
         return;

lib/Serialization/SerializedModuleLoader.cpp

@@ -720,7 +720,8 @@ void swift::serialization::diagnoseSerializedASTLoadFailure(
   case serialization::Status::FailedToLoadBridgingHeader:
     // We already emitted a diagnostic about the bridging header. Just emit
     // a generic message here.
-    Ctx.Diags.diagnose(diagLoc, diag::serialization_load_failed, ModuleName);
+    Ctx.Diags.diagnose(diagLoc, diag::serialization_load_failed,
+                       ModuleName.str());
     break;
 
   case serialization::Status::NameMismatch: {

test/ParseableInterface/ModuleCache/module-cache-diagnostics.swift

@@ -57,7 +57,9 @@
 // RUN: %{python} %S/Inputs/check-is-old.py %t/modulecache/OtherModule-*.swiftmodule %t/modulecache/LeafModule-*.swiftmodule
 // RUN: %FileCheck %s -check-prefix=CHECK-ERROR <%t/err.txt
 // CHECK-ERROR: LeafModule.swiftinterface:7:8: error: no such module 'NotAModule'
-// CHECK-ERROR: OtherModule.swiftinterface:4:8: error: no such module 'LeafModule'
+// CHECK-ERROR: OtherModule.swiftinterface:4:8: error: failed to load module 'LeafModule'
+// CHECK-ERROR: module-cache-diagnostics.swift:{{[0-9]+}}:8: error: failed to load module 'OtherModule'
+// CHECK-ERROR-NOT: error:
 //
 //
 // Next test: same as above, but with a .dia file
@@ -80,7 +82,9 @@
 // RUN: %{python} %S/Inputs/check-is-old.py %t/modulecache/OtherModule-*.swiftmodule %t/modulecache/LeafModule-*.swiftmodule
 // RUN: %FileCheck %s -check-prefix=CHECK-ERROR-INLINE <%t/err-inline.txt
 // CHECK-ERROR-INLINE: LeafModule.swiftinterface:6:33: error: use of unresolved identifier 'unresolved'
-// CHECK-ERROR-INLINE: OtherModule.swiftinterface:4:8: error: no such module 'LeafModule'
+// CHECK-ERROR-INLINE: OtherModule.swiftinterface:4:8: error: failed to load module 'LeafModule'
+// CHECK-ERROR-INLINE: module-cache-diagnostics.swift:{{[0-9]+}}:8: error: failed to load module 'OtherModule'
+// CHECK-ERROR-INLINE-NOT: error:
 //
 //
 // Next test: same as above, but with a .dia file

validation-test/ParseableInterface/Inputs/failing-overlay/HasOverlay.h

@@ -0,0 +1,4 @@
+// swift-interface-format-version: 1.0
+// swift-module-flags:
+
+garbage

validation-test/ParseableInterface/Inputs/failing-overlay/HasOverlay.swiftinterface

@@ -0,0 +1 @@
+#include <HasOverlay.h>

validation-test/ParseableInterface/Inputs/failing-overlay/ImportsOverlay.h

@@ -0,0 +1,8 @@
+module HasOverlay {
+  header "HasOverlay.h"
+}
+
+module ImportsOverlay {
+  header "ImportsOverlay.h"
+  export *
+}

validation-test/ParseableInterface/Inputs/failing-overlay/module.modulemap

@@ -0,0 +1,8 @@
+// RUN: %empty-directory(%t)
+// RUN: not %target-swift-frontend -I %S/Inputs/failing-overlay/ -typecheck %s 2>&1 | %FileCheck %s
+
+import ImportsOverlay
+
+// FIXME: It would be better if this had a useful location, like inside the
+// C header that caused the importing of the overlay.
+// CHECK: <unknown>:0: error: failed to load module 'HasOverlay'