Add all deserialization fatal output to the pretty stack trace

Rather than outputting diagnostics and to stderr, output all the extra
information added when deserialization fatally fails to the pretty stack
trace instead. Since the pretty stack trace is added to crash logs, this
should avoid the dance of requesting the compiler output

  - Moves the previous "**** DESERIALIZATION FAILURE ..." output to the
    last pretty stack trace line

  - Removes the module and compiler version notes added to the fatal
    diagnostic

  - Adds a new effective compiler version line for all frontend failure.
    Somewhat duplicates the line from the driver, but adds in the
    effective version

  - Adds a new line for the full misc version of the module that failed.
    May double up with previous "While reading from ..." lines that are
    added in various deserialization methods, but better to have it
    twice than not at all

Author: bnbarham

include/swift/AST/DiagnosticsSema.def

@@ -792,14 +792,6 @@ ERROR(serialization_fatal,Fatal,
       "fatal error encountered while reading from module '%0'; "
       SWIFT_BUG_REPORT_MESSAGE,
       (StringRef))
-NOTE(serialization_misc_version,none,
-     "module '%0' full misc version is '%1'"
-     "%select{ (built while allowing compiler errors)|}2",
-     (StringRef, StringRef, bool))
-NOTE(serialization_compatibility_version_mismatch,none,
-     "compiling as Swift %0, with '%1' built as Swift %2 "
-     "(this is supported but may expose additional compiler issues)",
-     (StringRef, StringRef, StringRef))
 
 ERROR(serialization_invalid_decl,Fatal,
       "deserialized invalid declaration %0 (%1) in module '%2'",

lib/FrontendTool/FrontendTool.cpp

@@ -48,6 +48,7 @@
 #include "swift/Basic/SourceManager.h"
 #include "swift/Basic/Statistic.h"
 #include "swift/Basic/UUID.h"
+#include "swift/Basic/Version.h"
 #include "swift/Option/Options.h"
 #include "swift/Frontend/Frontend.h"
 #include "swift/Frontend/AccumulatingDiagnosticConsumer.h"
@@ -1953,6 +1954,23 @@ static void printTargetInfo(const CompilerInvocation &invocation,
   out << "}\n";
 }
 
+/// A PrettyStackTraceEntry to print frontend information useful for debugging.
+class PrettyStackTraceFrontend : public llvm::PrettyStackTraceEntry {
+  const LangOptions &LangOpts;
+
+public:
+  PrettyStackTraceFrontend(const LangOptions &langOpts)
+      : LangOpts(langOpts) {}
+
+  void print(llvm::raw_ostream &os) const override {
+    auto effective = LangOpts.EffectiveLanguageVersion;
+    if (effective != version::Version::getCurrentLanguageVersion()) {
+      os << "Compiling with effective version " << effective;
+    }
+    os << "\n";
+  };
+};
+
 int swift::performFrontend(ArrayRef<const char *> Args,
                            const char *Argv0, void *MainAddr,
                            FrontendObserver *observer) {
@@ -2046,10 +2064,7 @@ int swift::performFrontend(ArrayRef<const char *> Args,
     return finishDiagProcessing(1, /*verifierEnabled*/ false);
   }
 
-  Optional<llvm::PrettyStackTraceString> allowErrorsStackTrace;
-  if (Invocation.getFrontendOptions().AllowModuleWithCompilerErrors)
-    allowErrorsStackTrace.emplace("While allowing modules with compiler errors "
-                                  "enabled");
+  PrettyStackTraceFrontend frontendTrace(Invocation.getLangOptions());
 
   // Make an array of PrettyStackTrace objects to dump the configuration files
   // we used to parse the arguments. These are RAII objects, so they and the

lib/Serialization/Deserialization.cpp

@@ -57,10 +57,6 @@ using namespace swift;
 using namespace swift::serialization;
 using llvm::Expected;
 
-StringRef swift::getNameOfModule(const ModuleFile *MF) {
-  return MF->getName();
-}
-
 namespace {
   struct DeclAndOffset {
     const Decl *D;
@@ -111,7 +107,7 @@ namespace {
           os << DeclAndOffset{DeclOrOffset.get(), offset};
         }
       }
-      os << " in '" << getNameOfModule(MF) << "'\n";
+      os << " in '" << MF->getName() << "'\n";
     }
   };
 
@@ -166,33 +162,15 @@ static void skipRecord(llvm::BitstreamCursor &cursor, unsigned recordKind) {
   (void)kind;
 }
 
-void ModuleFile::fatal(llvm::Error error) {
-  if (FileContext) {
-    getContext().Diags.diagnose(SourceLoc(), diag::serialization_fatal, Core->Name);
-    getContext().Diags.diagnose(
-        SourceLoc(), diag::serialization_misc_version, Core->Name,
-        Core->MiscVersion, allowCompilerErrors());
-
-    if (!Core->CompatibilityVersion.empty()) {
-      if (getContext().LangOpts.EffectiveLanguageVersion
-            != Core->CompatibilityVersion) {
-        SmallString<16> effectiveVersionBuffer, compatVersionBuffer;
-        {
-          llvm::raw_svector_ostream out(effectiveVersionBuffer);
-          out << getContext().LangOpts.EffectiveLanguageVersion;
-        }
-        {
-          llvm::raw_svector_ostream out(compatVersionBuffer);
-          out << Core->CompatibilityVersion;
-        }
-        getContext().Diags.diagnose(
-            SourceLoc(), diag::serialization_compatibility_version_mismatch,
-            effectiveVersionBuffer, Core->Name, compatVersionBuffer);
-      }
-    }
-  }
+void ModuleFile::fatal(llvm::Error error) const {
+  if (FileContext)
+    getContext().Diags.diagnose(SourceLoc(), diag::serialization_fatal,
+                                Core->Name);
+  Core->fatal(std::move(error));
+}
 
-  ModuleFileSharedCore::fatal(std::move(error));
+void ModuleFile::outputDiagnosticInfo(llvm::raw_ostream &os) const {
+  Core->outputDiagnosticInfo(os);
 }
 
 static Optional<swift::AccessorKind>
@@ -6302,7 +6280,7 @@ void ModuleFile::finishNormalConformance(NormalProtocolConformance *conformance,
                                          uint64_t contextData) {
   using namespace decls_block;
 
-  PrettyStackTraceModuleFile traceModule("While reading from", *this);
+  PrettyStackTraceModuleFile traceModule(*this);
   PrettyStackTraceConformance trace("finishing conformance for",
                                     conformance);
   ++NumNormalProtocolConformancesCompleted;
@@ -6718,9 +6696,3 @@ Optional<ForeignAsyncConvention> ModuleFile::maybeReadForeignAsyncConvention() {
       completionHandlerErrorFlagParamIndex,
       errorFlagPolarity);
 }
-
-void serialization::PrettyStackTraceModuleFile::outputModuleBuildInfo(
-    raw_ostream &os) const {
-  if (MF.compiledAllowingCompilerErrors())
-    os << " (built while allowing compiler errors)";
-}

lib/Serialization/DeserializationErrors.h

@@ -13,19 +13,15 @@
 #ifndef SWIFT_SERIALIZATION_DESERIALIZATIONERRORS_H
 #define SWIFT_SERIALIZATION_DESERIALIZATIONERRORS_H
 
+#include "ModuleFile.h"
+#include "ModuleFileSharedCore.h"
 #include "ModuleFormat.h"
 #include "swift/AST/Identifier.h"
 #include "swift/AST/Module.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/PrettyStackTrace.h"
 
 namespace swift {
-class ModuleFile;
-class ModuleFileSharedCore;
-
-StringRef getNameOfModule(const ModuleFile *);
-StringRef getNameOfModule(const ModuleFileSharedCore *);
-
 namespace serialization {
 
 class XRefTracePath {
@@ -470,12 +466,10 @@ class PrettyStackTraceModuleFile : public llvm::PrettyStackTraceEntry {
       : PrettyStackTraceModuleFile("While reading from", module) {}
 
   void print(raw_ostream &os) const override {
-    os << Action << " \'" << getNameOfModule(&MF) << "'";
-    outputModuleBuildInfo(os);
+    os << Action << " ";
+    MF.outputDiagnosticInfo(os);
     os << "\n";
   }
-
-  void outputModuleBuildInfo(raw_ostream &os) const;
 };
 
 class PrettyStackTraceModuleFileCore : public llvm::PrettyStackTraceEntry {
@@ -485,7 +479,9 @@ class PrettyStackTraceModuleFileCore : public llvm::PrettyStackTraceEntry {
       : MF(module) {}
 
   void print(raw_ostream &os) const override {
-    os << "While reading from \'" << getNameOfModule(&MF) << "'\n";
+    os << "While reading from ";
+    MF.outputDiagnosticInfo(os);
+    os << "\n";
   }
 };
 

lib/Serialization/ModuleFile.h

@@ -75,8 +75,6 @@ class ModuleFile
   llvm::BitstreamCursor SILIndexCursor;
   llvm::BitstreamCursor DeclMemberTablesCursor;
 
-  friend StringRef getNameOfModule(const ModuleFile *);
-
 public:
   static std::unique_ptr<llvm::MemoryBuffer> getModuleName(ASTContext &Ctx,
                                                            StringRef modulePath,
@@ -333,24 +331,27 @@ class ModuleFile
     return issue;
   }
 
-  /// Emits one last diagnostic, logs the error, and then aborts for the stack
-  /// trace.
-  LLVM_ATTRIBUTE_NORETURN void fatal(llvm::Error error);
-  void fatalIfNotSuccess(llvm::Error error) {
+  /// Emits one last diagnostic, adds the current module details and errors to
+  /// the pretty stack trace, and then aborts.
+  LLVM_ATTRIBUTE_NORETURN void fatal(llvm::Error error) const;
+  void fatalIfNotSuccess(llvm::Error error) const {
     if (error)
       fatal(std::move(error));
   }
-  template <typename T> T fatalIfUnexpected(llvm::Expected<T> expected) {
+  template <typename T> T fatalIfUnexpected(llvm::Expected<T> expected) const {
     if (expected)
       return std::move(expected.get());
     fatal(expected.takeError());
   }
 
-  LLVM_ATTRIBUTE_NORETURN void fatal() {
+  LLVM_ATTRIBUTE_NORETURN void fatal() const {
     fatal(llvm::make_error<llvm::StringError>(
         "(see \"While...\" info below)", llvm::inconvertibleErrorCode()));
   }
 
+  /// Outputs information useful for diagnostics to \p out
+  void outputDiagnosticInfo(llvm::raw_ostream &os) const;
+
   ASTContext &getContext() const {
     assert(FileContext && "no associated context yet");
     return FileContext->getParentModule()->getASTContext();

lib/Serialization/ModuleFileSharedCore.cpp

@@ -14,19 +14,17 @@
 #include "ModuleFileCoreTableInfo.h"
 #include "BCReadingExtras.h"
 #include "DeserializationErrors.h"
+#include "swift/Basic/LangOptions.h"
 #include "swift/Strings.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/OnDiskHashTable.h"
+#include "llvm/Support/PrettyStackTrace.h"
 
 using namespace swift;
 using namespace swift::serialization;
 using namespace llvm::support;
 using llvm::Expected;
 
-StringRef swift::getNameOfModule(const ModuleFileSharedCore *MF) {
-  return MF->getName();
-}
-
 static bool checkModuleSignature(llvm::BitstreamCursor &cursor,
                                  ArrayRef<unsigned char> signature) {
   for (unsigned char byte : signature) {
@@ -472,13 +470,31 @@ std::string ModuleFileSharedCore::Dependency::getPrettyPrintedPath() const {
   return output;
 }
 
-void ModuleFileSharedCore::fatal(llvm::Error error) {
-  logAllUnhandledErrors(std::move(error), llvm::errs(),
-                        "\n*** DESERIALIZATION FAILURE (please include this "
-                        "section in any bug report) ***\n");
+void ModuleFileSharedCore::fatal(llvm::Error error) const {
+  llvm::SmallString<0> errorStr;
+  llvm::raw_svector_ostream out(errorStr);
+
+  out << "*** DESERIALIZATION FAILURE ***\n";
+  outputDiagnosticInfo(out);
+  out << "\n";
+  if (error) {
+    handleAllErrors(std::move(error), [&](const llvm::ErrorInfoBase &ei) {
+      ei.log(out);
+      out << "\n";
+    });
+  }
+
+  llvm::PrettyStackTraceString trace(errorStr.c_str());
   abort();
 }
 
+void ModuleFileSharedCore::outputDiagnosticInfo(llvm::raw_ostream &os) const {
+  os << "module '" << Name << "' with full misc version '" << MiscVersion
+      << "'";
+  if (Bits.IsAllowModuleWithCompilerErrorsEnabled)
+    os << " (built with -experimental-allow-module-with-compiler-errors)";
+}
+
 ModuleFileSharedCore::~ModuleFileSharedCore() { }
 
 std::unique_ptr<ModuleFileSharedCore::SerializedDeclTable>

lib/Serialization/ModuleFileSharedCore.h

@@ -371,12 +371,12 @@ class ModuleFileSharedCore {
 
   /// Emits one last diagnostic, logs the error, and then aborts for the stack
   /// trace.
-  LLVM_ATTRIBUTE_NORETURN static void fatal(llvm::Error error);
-  void fatalIfNotSuccess(llvm::Error error) {
+  LLVM_ATTRIBUTE_NORETURN void fatal(llvm::Error error) const;
+  void fatalIfNotSuccess(llvm::Error error) const {
     if (error)
       fatal(std::move(error));
   }
-  template <typename T> T fatalIfUnexpected(llvm::Expected<T> expected) {
+  template <typename T> T fatalIfUnexpected(llvm::Expected<T> expected) const {
     if (expected)
       return std::move(expected.get());
     fatal(expected.takeError());
@@ -508,6 +508,9 @@ class ModuleFileSharedCore {
     return info;
   }
 
+  /// Outputs information useful for diagnostics to \p out
+  void outputDiagnosticInfo(llvm::raw_ostream &os) const;
+  
   // Out of line to avoid instantiation OnDiskChainedHashTable here.
   ~ModuleFileSharedCore();
 

test/Frontend/crash-in-user-code.swift

@@ -12,6 +12,7 @@
 // CHECK: Stack dump:
 // CHECK-NEXT: Program arguments:
 // CHECK-NEXT: Swift version
+// CHECK-NEXT: Compiling with effective version
 // CHECK-NEXT: Contents of {{.*}}.filelist.txt:
 // CHECK-NEXT: ---
 // CHECK-NEXT: crash-in-user-code.swift

test/Frontend/crash.swift

@@ -2,18 +2,19 @@
 // RUN: not --crash %target-swift-frontend -typecheck -debug-crash-after-parse -filelist %t.filelist.txt 2>&1 | %FileCheck %s
 
 // Check that we see the contents of the input file list in the crash log.
+// CHECK-NOT: Compiling with {{.*}} while allowing modules with compiler errors enabled
 // CHECK-LABEL: Stack dump
 // CHECK-NEXT: Program arguments: {{.*swift(-frontend)?(c?)(\.exe)?}}
 // CHECK-NEXT: Swift version
+// CHECK-NEXT: Compiling with effective version
 // CHECK-NEXT: Contents of {{.*}}.filelist.txt:
 // CHECK-NEXT: ---
 // CHECK-NEXT: test{{[\\/]}}Frontend{{[\\/]}}crash.swift{{$}}
 // CHECK-NEXT: ---
 
-// Check that a message when allowing errors is output
 // RUN: not --crash %target-swift-frontend -typecheck -debug-crash-after-parse -experimental-allow-module-with-compiler-errors %s 2>&1 | %FileCheck -check-prefix CHECK-ALLOW %s
-// CHECK-ALLOW-LABEL: Stack dump
-// CHECK-ALLOW: While allowing modules with compiler errors enabled
+// CHECK-ALLOW: Program arguments: {{.*}} -experimental-allow-module-with-compiler-errors
+// CHECK-ALLOW: Compiling with effective version
 
 func anchor() {}
 anchor()

test/Serialization/AllowErrors/crash-adds-message.swift

@@ -13,9 +13,9 @@ public class Sub: Base {
 }
 
 // CHECK-CRASH: error: fatal error encountered while reading from module 'Lib'; please submit a bug report (https://swift.org/contributing/#reporting-bugs) and include the project
-// CHECK-CRASH: note: module 'Lib' full misc version is
-// CHECK-CRASH-4-NOT: note: compiling as
-// CHECK-CRASH-4_2: note: compiling as Swift 4.2, with 'Lib' built as Swift 4.1.50
-// CHECK-CRASH-LABEL: *** DESERIALIZATION FAILURE (please include this section in any bug report) ***
-// CHECK-CRASH: could not find 'disappearingMethod()' in parent class
+// CHECK-CRASH-4: Compiling with effective version 4.1.50
+// CHECK-CRASH-4_2: Compiling with effective version 4.2
 // CHECK-CRASH: While loading members for 'Sub' (in module 'Lib')
+// CHECK-CRASH-LABEL: *** DESERIALIZATION FAILURE ***
+// CHECK-CRASH: module 'Lib' with full misc version {{.*}}4.1.50
+// CHECK-CRASH: could not find 'disappearingMethod()' in parent class