Merge pull request #8666 from jrose-apple/more-experimental-deserialization-recovery

More -enable-experimental-deserialization-recovery

Author: jrose-apple

include/swift/AST/DiagnosticsSema.def

@@ -553,6 +553,10 @@ ERROR(serialization_fatal,Fatal,
       "fatal error encountered while reading from module '%0'; "
       "please file a bug report with your project and the crash log",
       (StringRef))
+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(reserved_member_name,none,
       "type member may not be named %0, since it would conflict with the"

include/swift/Serialization/ModuleFile.h

@@ -72,6 +72,9 @@ class ModuleFile : public LazyMemberLoader {
   /// The target the module was built for.
   StringRef TargetTriple;
 
+  /// The Swift compatibility version in use when this module was built.
+  StringRef CompatibilityVersion;
+
   /// The data blob containing all of the module's identifiers.
   StringRef IdentifierData;
 

include/swift/Serialization/ModuleFormat.h

@@ -443,6 +443,7 @@ namespace control_block {
     BCFixed<16>, // Module format major version
     BCFixed<16>, // Module format minor version
     BCVBR<8>, // length of "short version string" in the blob
+    BCVBR<8>, // length of "short compatibility version string" in the blob
     BCBlob // misc. version information
   >;
 

include/swift/Serialization/Validation.h

@@ -71,6 +71,7 @@ struct ValidationInfo {
   StringRef name = {};
   StringRef targetTriple = {};
   StringRef shortVersion = {};
+  StringRef compatibilityVersion = {};
   size_t bytes = 0;
   Status status = Status::Malformed;
 };

lib/Serialization/Deserialization.cpp

@@ -344,6 +344,17 @@ static bool skipRecord(llvm::BitstreamCursor &cursor, unsigned recordKind) {
 void ModuleFile::fatal(llvm::Error error) {
   if (FileContext) {
     getContext().Diags.diagnose(SourceLoc(), diag::serialization_fatal, Name);
+
+    if (!CompatibilityVersion.empty()) {
+      SmallString<16> buffer;
+      llvm::raw_svector_ostream out(buffer);
+      out << getContext().LangOpts.EffectiveLanguageVersion;
+      if (out.str() != CompatibilityVersion) {
+        getContext().Diags.diagnose(
+            SourceLoc(), diag::serialization_compatibility_version_mismatch,
+            out.str(), Name, CompatibilityVersion);
+      }
+    }
   }
 
   logAllUnhandledErrors(std::move(error), llvm::errs(),
@@ -1231,9 +1242,7 @@ bool ModuleFile::readMembers(SmallVectorImpl<Decl *> &Members) {
       if (!getContext().LangOpts.EnableDeserializationRecovery)
         fatal(D.takeError());
 
-      // Silently drop the member if there was a problem.
-      // FIXME: This isn't sound for protocols; we need to at least record that
-      // it happened.
+      // Silently drop the member if it had an override-related problem.
       llvm::handleAllErrors(D.takeError(),
                             [](const OverrideError &) { /* expected */ },
                             [&](std::unique_ptr<llvm::ErrorInfoBase> unhandled){

lib/Serialization/ModuleFile.cpp

@@ -193,9 +193,20 @@ validateControlBlock(llvm::BitstreamCursor &cursor,
         }
       }
 
-      // This field was added later; be resilient against its absence.
-      if (scratch.size() > 2) {
+      // These fields were added later; be resilient against their absence.
+      switch (scratch.size()) {
+      default:
+        // Add new cases here, in descending order.
+      case 4:
+        result.compatibilityVersion = blobData.substr(scratch[2]+1, scratch[3]);
+        LLVM_FALLTHROUGH;
+      case 3:
         result.shortVersion = blobData.slice(0, scratch[2]);
+        LLVM_FALLTHROUGH;
+      case 2:
+      case 1:
+      case 0:
+        break;
       }
 
       versionSeen = true;
@@ -932,6 +943,7 @@ ModuleFile::ModuleFile(
       }
       Name = info.name;
       TargetTriple = info.targetTriple;
+      CompatibilityVersion = info.compatibilityVersion;
 
       hasValidControlBlock = true;
       break;

lib/Serialization/Serialization.cpp

@@ -709,10 +709,14 @@ void Serializer::writeHeader(const SerializationOptions &options) {
     llvm::raw_svector_ostream versionString(versionStringBuf);
     versionString << Version::getCurrentLanguageVersion();
     size_t shortVersionStringLength = versionString.tell();
-    versionString << '/' << version::getSwiftFullVersion(
-      M->getASTContext().LangOpts.EffectiveLanguageVersion);
+    versionString << '('
+                  << M->getASTContext().LangOpts.EffectiveLanguageVersion;
+    size_t compatibilityVersionStringLength =
+        versionString.tell() - shortVersionStringLength - 1;
+    versionString << ")/" << version::getSwiftFullVersion();
     Metadata.emit(ScratchRecord,
                   VERSION_MAJOR, VERSION_MINOR, shortVersionStringLength,
+                  compatibilityVersionStringLength,
                   versionString.str());
 
     Target.emit(ScratchRecord, M->getASTContext().LangOpts.Target.str());
@@ -772,7 +776,7 @@ void Serializer::writeDocHeader() {
     auto& LangOpts = M->getASTContext().LangOpts;
     Metadata.emit(ScratchRecord,
                   VERSION_MAJOR, VERSION_MINOR,
-                  /*short version string length*/0,
+                  /*short version string length*/0, /*compatibility length*/0,
                   version::getSwiftFullVersion(
                     LangOpts.EffectiveLanguageVersion));
 

test/Serialization/Recovery/Inputs/custom-modules/Overrides.h

@@ -1,5 +1,35 @@
-@interface Base
-#ifndef BAD
-- (void)method;
+@interface Object
+- (nonnull instancetype)init;
+@end
+
+@interface Base : Object
+#if !BAD
+- (void)disappearingMethod;
+- (nullable id)nullabilityChangeMethod;
+- (nonnull id)typeChangeMethod;
+#else
+//- (void)disappearingMethod;
+- (nonnull id)nullabilityChangeMethod;
+- (nonnull Base *)typeChangeMethod;
+#endif
+@end
+
+@interface Base (ExtraMethodsToTriggerCircularReferences)
+#if !BAD
+- (void)disappearingMethodWithOverload;
+#else
+//- (void)disappearingMethodWithOverload;
+#endif
+@end
+
+@interface GenericBase<Element>: Object
+#if !BAD
+- (void)disappearingMethod;
+- (nullable Element)nullabilityChangeMethod;
+- (nonnull id)typeChangeMethod;
+#else
+//- (void)disappearingMethod;
+- (nonnull Element)nullabilityChangeMethod;
+- (nonnull Element)typeChangeMethod;
 #endif
 @end

test/Serialization/Recovery/crash-recovery.swift

@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: %target-swift-frontend -emit-module -o %t -module-name Lib -I %S/Inputs/custom-modules -swift-version 3 %s
+
+// RUN: not --crash %target-swift-ide-test -source-filename=x -print-module -module-to-print Lib -I %t -I %S/Inputs/custom-modules -swift-version 3 -Xcc -DBAD 2>&1 | %FileCheck -check-prefix CHECK-CRASH -check-prefix CHECK-CRASH-3 %s
+// RUN: not --crash %target-swift-ide-test -source-filename=x -print-module -module-to-print Lib -I %t -I %S/Inputs/custom-modules -swift-version 4 -Xcc -DBAD 2>&1 | %FileCheck -check-prefix CHECK-CRASH -check-prefix CHECK-CRASH-4 %s
+
+// REQUIRES: objc_interop
+
+import Overrides
+
+public class Sub: Base {
+  public override func disappearingMethod() {}
+}
+
+// CHECK-CRASH: error: fatal error encountered while reading from module 'Lib'; please file a bug report with your project and the crash log
+// CHECK-CRASH-3-NOT: note
+// CHECK-CRASH-4: note: compiling as Swift 4.0, with 'Lib' built as Swift 3.1
+// CHECK-CRASH-LABEL: *** DESERIALIZATION FAILURE (please include this section in any bug report) ***
+// CHECK-CRASH: could not find 'disappearingMethod()' in parent class
+// CHECK-CRASH: While loading members for 'Sub' in module 'Lib'

test/Serialization/Recovery/overrides.swift

@@ -3,25 +3,53 @@
 
 // RUN: %target-swift-ide-test -source-filename=x -print-module -module-to-print Lib -I %t -I %S/Inputs/custom-modules | %FileCheck %s
 
-// RUN: not --crash %target-swift-ide-test -source-filename=x -print-module -module-to-print Lib -I %t -I %S/Inputs/custom-modules -Xcc -DBAD 2>&1 | %FileCheck -check-prefix CHECK-CRASH %s
 // RUN: %target-swift-ide-test -source-filename=x -print-module -module-to-print Lib -I %t -I %S/Inputs/custom-modules -Xcc -DBAD -enable-experimental-deserialization-recovery | %FileCheck -check-prefix CHECK-RECOVERY %s
 
 // REQUIRES: objc_interop
 
 import Overrides
 
-public class Sub: Base {
-  public override func method() {}
+// Please use prefixes to keep the printed parts of this file in alphabetical
+// order.
+
+public class SwiftOnlyClass {}
+
+public class A_Sub: Base {
+  public override func disappearingMethod() {}
+  public override func nullabilityChangeMethod() -> Any? { return nil }
+  public override func typeChangeMethod() -> Any { return self }
+  public override func disappearingMethodWithOverload() {}
 }
 
-// CHECK-LABEL: class Sub : Base {
-// CHECK-NEXT: func method()
+// CHECK-LABEL: class A_Sub : Base {
+// CHECK-NEXT: func disappearingMethod()
+// CHECK-NEXT: func nullabilityChangeMethod() -> Any?
+// CHECK-NEXT: func typeChangeMethod() -> Any
+// CHECK-NEXT: func disappearingMethodWithOverload()
+// CHECK-NEXT: init()
 // CHECK-NEXT: {{^}$}}
 
-// CHECK-CRASH: error: fatal error encountered while reading from module 'Lib'; please file a bug report with your project and the crash log
-// CHECK-CRASH-LABEL: *** DESERIALIZATION FAILURE (please include this section in any bug report) ***
-// CHECK-CRASH: could not find 'method()' in parent class
-// CHECK-CRASH: While loading members for 'Sub' in module 'Lib'
+// CHECK-RECOVERY-LABEL: class A_Sub : Base {
+// CHECK-RECOVERY-NEXT: init()
+// CHECK-RECOVERY-NEXT: {{^}$}}
+
+extension Base {
+  @nonobjc func disappearingMethodWithOverload() -> SwiftOnlyClass? { return nil }
+}
+
+public class B_GenericSub : GenericBase<Base> {
+  public override func disappearingMethod() {}
+  public override func nullabilityChangeMethod() -> Base? { return nil }
+  public override func typeChangeMethod() -> Any { return self }
+}
+
+// CHECK-LABEL: class B_GenericSub : GenericBase<Base> {
+// CHECK-NEXT: func disappearingMethod()
+// CHECK-NEXT: func nullabilityChangeMethod() -> Base?
+// CHECK-NEXT: func typeChangeMethod() -> Any
+// CHECK-NEXT: init()
+// CHECK-NEXT: {{^}$}}
 
-// CHECK-RECOVERY-LABEL: class Sub : Base {
+// CHECK-RECOVERY-LABEL: class B_GenericSub : GenericBase<Base> {
+// CHECK-RECOVERY-NEXT: init()
 // CHECK-RECOVERY-NEXT: {{^}$}}