Improve diagnostic for broken module interfaces

Currently, when a swiftinterface file fails to load, we emit the specific diagnostics for the failures, followed by a generic “failed to load module ‘Foo’” message. This PR improves that final diagnostic, particularly when the cause may be that the interface was emitted by a newer compiler using backwards-incompatible syntax.

Author: beccadax

include/swift/AST/DiagnosticsSema.def

@@ -686,6 +686,13 @@ ERROR(sema_opening_import,Fatal,
 
 ERROR(serialization_load_failed,Fatal,
       "failed to load module '%0'", (StringRef))
+ERROR(module_interface_build_failed,Fatal,
+      "failed to build module '%0' from its module interface; "
+      "%select{the compiler that produced it, '%2', may have used features "
+      "that aren't supported by this compiler, '%3'"
+      "|it may have been damaged or it may have triggered a bug in the Swift "
+      "compiler when it was produced}1",
+      (StringRef, bool, StringRef, StringRef))
 ERROR(serialization_malformed_module,Fatal,
       "malformed compiled module: %0", (StringRef))
 ERROR(serialization_module_too_new,Fatal,

include/swift/Frontend/ModuleInterfaceSupport.h

@@ -23,6 +23,7 @@
 
 namespace swift {
 
+class ASTContext;
 class ModuleDecl;
 
 /// Options for controlling the generation of the .swiftinterface output.
@@ -45,8 +46,10 @@ struct ModuleInterfaceOptions {
 };
 
 extern version::Version InterfaceFormatVersion;
+std::string getSwiftInterfaceCompilerVersionForCurrentCompiler(ASTContext &ctx);
 
 llvm::Regex getSwiftInterfaceFormatVersionRegex();
+llvm::Regex getSwiftInterfaceCompilerVersionRegex();
 llvm::Regex getSwiftInterfaceModuleFlagsRegex();
 
 /// Emit a stable module interface for \p M, which can be used by a client

lib/Frontend/ModuleInterfaceBuilder.cpp

@@ -135,8 +135,8 @@ void ModuleInterfaceBuilder::configureSubInvocation(
 }
 
 bool ModuleInterfaceBuilder::extractSwiftInterfaceVersionAndArgs(
-    swift::version::Version &Vers, llvm::StringSaver &SubArgSaver,
-    SmallVectorImpl<const char *> &SubArgs) {
+    swift::version::Version &Vers, StringRef &CompilerVersion,
+    llvm::StringSaver &SubArgSaver, SmallVectorImpl<const char *> &SubArgs) {
   auto FileOrError = swift::vfs::getFileOrSTDIN(fs, interfacePath);
   if (!FileOrError) {
     diags.diagnose(diagnosticLoc, diag::error_open_input_file,
@@ -145,8 +145,9 @@ bool ModuleInterfaceBuilder::extractSwiftInterfaceVersionAndArgs(
   }
   auto SB = FileOrError.get()->getBuffer();
   auto VersRe = getSwiftInterfaceFormatVersionRegex();
+  auto CompRe = getSwiftInterfaceCompilerVersionRegex();
   auto FlagRe = getSwiftInterfaceModuleFlagsRegex();
-  SmallVector<StringRef, 1> VersMatches, FlagMatches;
+  SmallVector<StringRef, 1> VersMatches, FlagMatches, CompMatches;
   if (!VersRe.match(SB, &VersMatches)) {
     diags.diagnose(diagnosticLoc,
                    diag::error_extracting_version_from_module_interface);
@@ -161,6 +162,16 @@ bool ModuleInterfaceBuilder::extractSwiftInterfaceVersionAndArgs(
   assert(FlagMatches.size() == 2);
   Vers = swift::version::Version(VersMatches[1], SourceLoc(), &diags);
   llvm::cl::TokenizeGNUCommandLine(FlagMatches[1], SubArgSaver, SubArgs);
+
+  if (CompRe.match(SB, &CompMatches)) {
+    assert(CompMatches.size() == 2);
+    CompilerVersion = SubArgSaver.save(CompMatches[1]);
+  }
+  else {
+    // Don't diagnose; handwritten module interfaces don't include this field.
+    CompilerVersion = "(unspecified, file possibly handwritten)";
+  }
+
   return false;
 }
 
@@ -274,7 +285,9 @@ bool ModuleInterfaceBuilder::buildSwiftModuleInternal(
     llvm::StringSaver SubArgSaver(SubArgsAlloc);
     SmallVector<const char *, 16> SubArgs;
     swift::version::Version Vers;
-    if (extractSwiftInterfaceVersionAndArgs(Vers, SubArgSaver, SubArgs)) {
+    StringRef emittedByCompiler;
+    if (extractSwiftInterfaceVersionAndArgs(Vers, emittedByCompiler,
+                                            SubArgSaver, SubArgs)) {
       SubError = true;
       return;
     }
@@ -325,8 +338,12 @@ bool ModuleInterfaceBuilder::buildSwiftModuleInternal(
       // 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);
+        auto builtByCompiler =
+            getSwiftInterfaceCompilerVersionForCurrentCompiler(
+                SubInstance.getASTContext());
+        diags.diagnose(diagnosticLoc, diag::module_interface_build_failed,
+                       moduleName, emittedByCompiler == builtByCompiler,
+                       emittedByCompiler, builtByCompiler);
       }
     };
 

lib/Frontend/ModuleInterfaceBuilder.h

@@ -66,8 +66,8 @@ class ModuleInterfaceBuilder {
       bool IsHashBased);
 
   bool extractSwiftInterfaceVersionAndArgs(
-      version::Version &Vers, llvm::StringSaver &SubArgSaver,
-      SmallVectorImpl<const char *> &SubArgs);
+      version::Version &Vers, StringRef &CompilerVersion,
+      llvm::StringSaver &SubArgSaver, SmallVectorImpl<const char *> &SubArgs);
 
   bool buildSwiftModuleInternal(StringRef OutPath, bool ShouldSerializeDeps,
                                 std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer);

lib/Frontend/ModuleInterfaceSupport.cpp

@@ -61,8 +61,8 @@ static void printToolVersionAndFlagsComment(raw_ostream &out,
                                             ModuleInterfaceOptions const &Opts,
                                             ModuleDecl *M) {
   auto &Ctx = M->getASTContext();
-  auto ToolsVersion = swift::version::getSwiftFullVersion(
-      Ctx.LangOpts.EffectiveLanguageVersion);
+  auto ToolsVersion =
+      getSwiftInterfaceCompilerVersionForCurrentCompiler(Ctx);
   out << "// " SWIFT_INTERFACE_FORMAT_VERSION_KEY ": "
       << InterfaceFormatVersion << "\n";
   out << "// " SWIFT_COMPILER_VERSION_KEY ": "
@@ -71,6 +71,12 @@ static void printToolVersionAndFlagsComment(raw_ostream &out,
       << Opts.Flags << "\n";
 }
 
+std::string
+swift::getSwiftInterfaceCompilerVersionForCurrentCompiler(ASTContext &ctx) {
+  return swift::version::getSwiftFullVersion(
+             ctx.LangOpts.EffectiveLanguageVersion);
+}
+
 llvm::Regex swift::getSwiftInterfaceFormatVersionRegex() {
   return llvm::Regex("^// " SWIFT_INTERFACE_FORMAT_VERSION_KEY
                      ": ([0-9\\.]+)$", llvm::Regex::Newline);
@@ -81,6 +87,11 @@ llvm::Regex swift::getSwiftInterfaceModuleFlagsRegex() {
                      llvm::Regex::Newline);
 }
 
+llvm::Regex swift::getSwiftInterfaceCompilerVersionRegex() {
+  return llvm::Regex("^// " SWIFT_COMPILER_VERSION_KEY
+                     ": (.+)$", llvm::Regex::Newline);
+}
+
 /// Prints the imported modules in \p M to \p out in the form of \c import
 /// source declarations.
 static void printImports(raw_ostream &out,

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

@@ -57,8 +57,8 @@
 // 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: failed to load module 'LeafModule'
-// CHECK-ERROR: module-cache-diagnostics.swift:{{[0-9]+}}:8: error: failed to load module 'OtherModule'
+// CHECK-ERROR: OtherModule.swiftinterface:4:8: error: failed to build module 'LeafModule' from its module interface; it may have been damaged or it may have triggered a bug in the Swift compiler when it was produced
+// CHECK-ERROR: module-cache-diagnostics.swift:{{[0-9]+}}:8: error: failed to build module 'OtherModule' from its module interface; it may have been damaged or it may have triggered a bug in the Swift compiler when it was produced
 // CHECK-ERROR-NOT: error:
 //
 //
@@ -82,8 +82,8 @@
 // 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: failed to load module 'LeafModule'
-// CHECK-ERROR-INLINE: module-cache-diagnostics.swift:{{[0-9]+}}:8: error: failed to load module 'OtherModule'
+// CHECK-ERROR-INLINE: OtherModule.swiftinterface:4:8: error: failed to build module 'LeafModule' from its module interface; it may have been damaged or it may have triggered a bug in the Swift compiler when it was produced
+// CHECK-ERROR-INLINE: module-cache-diagnostics.swift:{{[0-9]+}}:8: error: failed to build module 'OtherModule' from its module interface; it may have been damaged or it may have triggered a bug in the Swift compiler when it was produced
 // CHECK-ERROR-INLINE-NOT: error:
 //
 //

test/ModuleInterface/unbuildable.swift

@@ -0,0 +1,27 @@
+// Tests the diagnostics emitted when we can't build a module interface, and
+// particularly whether or not they call out compiler version mismatches.
+//
+// First, emit an empty module interface from the current compiler, then add
+// some invalid syntax to it:
+//
+// RUN: %empty-directory(%t)
+// RUN: echo "" | %target-swift-frontend -typecheck -emit-module-interface-path %t/UnbuildableCurrent.swiftinterface -enable-library-evolution -swift-version 5 -module-name UnbuildableCurrent -
+// RUN: echo "public func fn(_: String = #somethingYouveNeverHeardOf)" >> %t/UnbuildableCurrent.swiftinterface
+//
+// Next, modify a version of it so it looks like it was built with a different Swift compiler:
+//
+// RUN: sed -E -e 's|swift-compiler-version: (.*)|swift-compiler-version: NeoTokyoSwift 2000.42|' -e 's|UnbuildableCurrent|UnbuildableFuture|g' %t/UnbuildableCurrent.swiftinterface > %t/UnbuildableFuture.swiftinterface
+//
+// And finally, test:
+//
+// RUN: not %target-swift-frontend -typecheck %s -I %t -DCURRENT 2>&1 | %FileCheck -check-prefix=CURRENT %s
+// RUN: not %target-swift-frontend -typecheck %s -I %t -DFUTURE 2>&1 | %FileCheck -check-prefix=FUTURE %s
+
+#if CURRENT
+import UnbuildableCurrent
+// CURRENT: unbuildable.swift:[[@LINE-1]]:8: error: failed to build module 'UnbuildableCurrent' from its module interface; it may have been damaged or it may have triggered a bug in the Swift compiler when it was produced
+#else
+import UnbuildableFuture
+// FUTURE: unbuildable.swift:[[@LINE-1]]:8: error: failed to build module 'UnbuildableFuture' from its module interface; the compiler that produced it, 'NeoTokyoSwift 2000.42', may have used features that aren't supported by this compiler, '{{.*Swift version.*}}'
+#endif
+

validation-test/ParseableInterface/failing-overlay.swift

@@ -5,4 +5,4 @@ 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'
+// CHECK: <unknown>:0: error: failed to build module 'HasOverlay' from its module interface; the compiler that produced it, '(unspecified, file possibly handwritten)', may have used features that aren't supported by this compiler, '{{.*}}Swift version{{.*}}'