Serialization: Recovery for protocol conformances with changed witness or requirement signatures.

Deserializing a witness record in a conformance may fail if either of the requirement or witness changed name or type, most likely due to SDK modernization changes across Swift versions. When this happens, leave an opaque placeholder in the conformance to indicate that the witness exists but we don't get to see it. For expedience, right now this just witnesses the requirement to itself, so that code in the type checker or elsewhere that tries to ad-hoc devirtualize references to the requirement just gets the requirement back. Arguably, we shouldn't include the witness at all in imported conformances, since they should be an implementation detail, but that's a bigger, riskier change. This patch as is should be enough to address rdar://problem/31185053.

Author: jckarter

include/swift/AST/Witness.h

@@ -108,6 +108,16 @@ class Witness {
   /// not generic (excepting \c Self)  and the conforming type is non-generic.
   Witness(ValueDecl *witness) : storage(witness) { assert(witness != nullptr); }
 
+  /// Create an opaque witness for the given requirement.
+  ///
+  /// This indicates that a witness exists, but is not visible to the current
+  /// module.
+  static Witness forOpaque(ValueDecl *requirement) {
+    // TODO: It's probably a good idea to have a separate 'opaque' bit.
+    // Making req == witness is kind of a hack.
+    return Witness(requirement);
+  }
+
   /// Create a witness that requires substitutions.
   ///
   /// \param decl The declaration for the witness.

include/swift/Basic/Version.h

@@ -144,6 +144,9 @@ class Version {
 
 bool operator>=(const Version &lhs, const Version &rhs);
 bool operator==(const Version &lhs, const Version &rhs);
+inline bool operator!=(const Version &lhs, const Version &rhs) {
+  return !(lhs == rhs);
+}
 
 raw_ostream &operator<<(raw_ostream &os, const Version &version);
 

include/swift/Serialization/ModuleFile.h

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

include/swift/Serialization/Validation.h

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

lib/Basic/Version.cpp

@@ -226,6 +226,11 @@ Optional<Version> Version::parseVersionString(StringRef VersionString,
   return isValidVersion ? Optional<Version>(TheVersion) : None;
 }
 
+Version::Version(StringRef VersionString,
+                 SourceLoc Loc,
+                 DiagnosticEngine *Diags)
+  : Version(*parseVersionString(VersionString, Loc, Diags))
+{}
 
 Version Version::getCurrentCompilerVersion() {
 #ifdef SWIFT_COMPILER_VERSION

lib/Serialization/Deserialization.cpp

@@ -165,13 +165,20 @@ void ModuleFile::fatal(llvm::Error error) {
     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) {
+      if (getContext().LangOpts.EffectiveLanguageVersion
+            != CompatibilityVersion) {
+        SmallString<16> effectiveVersionBuffer, compatVersionBuffer;
+        {
+          llvm::raw_svector_ostream out(effectiveVersionBuffer);
+          out << getContext().LangOpts.EffectiveLanguageVersion;
+        }
+        {
+          llvm::raw_svector_ostream out(compatVersionBuffer);
+          out << CompatibilityVersion;
+        }
         getContext().Diags.diagnose(
             SourceLoc(), diag::serialization_compatibility_version_mismatch,
-            out.str(), Name, CompatibilityVersion);
+            effectiveVersionBuffer, Name, compatVersionBuffer);
       }
     }
   }
@@ -4464,20 +4471,69 @@ void ModuleFile::finishNormalConformance(NormalProtocolConformance *conformance,
 
   ArrayRef<uint64_t>::iterator rawIDIter = rawIDs.begin();
 
+  // An imported requirement may have changed type between Swift versions.
+  // In this situation we need to do a post-pass to fill in missing
+  // requirements with opaque witnesses.
+  bool needToFillInOpaqueValueWitnesses = false;
   while (valueCount--) {
-    auto req = cast<ValueDecl>(getDecl(*rawIDIter++));
-    auto witness = cast_or_null<ValueDecl>(getDecl(*rawIDIter++));
-    assert(witness ||
+    ValueDecl *req;
+    
+    auto trySetWitness = [&](Witness w) {
+      if (req)
+        conformance->setWitness(req, w);
+    };
+    
+    auto deserializedReq = getDeclChecked(*rawIDIter++);
+    if (deserializedReq) {
+      req = cast<ValueDecl>(*deserializedReq);
+    } else if (getContext().LangOpts.EnableDeserializationRecovery) {
+      consumeError(deserializedReq.takeError());
+      req = nullptr;
+      needToFillInOpaqueValueWitnesses = true;
+    } else {
+      fatal(deserializedReq.takeError());
+    }
+    
+    bool isOpaque = false;
+    ValueDecl *witness;
+    auto deserializedWitness = getDeclChecked(*rawIDIter++);
+    if (deserializedWitness) {
+      witness = cast<ValueDecl>(*deserializedWitness);
+    // Across language compatibility versions, the witnessing decl may have
+    // changed its signature as seen by the current compatibility version.
+    // In that case, we want the conformance to still be available, but
+    // we can't make use of the relationship to the underlying decl.
+    } else if (getContext().LangOpts.EnableDeserializationRecovery) {
+      consumeError(deserializedWitness.takeError());
+      isOpaque = true;
+      witness = nullptr;
+    } else {
+      fatal(deserializedWitness.takeError());
+    }
+    
+    assert(!req || isOpaque || witness ||
            req->getAttrs().hasAttribute<OptionalAttr>() ||
            req->getAttrs().isUnavailable(getContext()));
-    if (!witness) {
-      conformance->setWitness(req, Witness());
+    if (!witness && !isOpaque) {
+      trySetWitness(Witness());
       continue;
     }
 
     // Generic signature and environment.
     GenericSignature *syntheticSig = nullptr;
     GenericEnvironment *syntheticEnv = nullptr;
+    
+    auto trySetOpaqueWitness = [&]{
+      if (!req)
+        return;
+      
+      // We shouldn't yet need to worry about generic requirements, since
+      // an imported ObjC method should never be generic.
+      assert(syntheticSig == nullptr && syntheticEnv == nullptr &&
+             "opaque witness shouldn't be generic yet. when this is "
+             "possible, it should use forwarding substitutions");
+      conformance->setWitness(req, Witness::forOpaque(req));
+    };
 
     // Requirement -> synthetic map.
     SmallVector<Substitution, 4> reqToSyntheticSubs;
@@ -4518,16 +4574,22 @@ void ModuleFile::finishNormalConformance(NormalProtocolConformance *conformance,
       }
     }
 
+    // Handle opaque witnesses that couldn't be deserialized.
+    if (isOpaque) {
+      trySetOpaqueWitness();
+      continue;
+    }
+
     // Handle simple witnesses.
     if (witnessSubstitutions.empty() && !syntheticSig && !syntheticEnv &&
         reqToSyntheticSubs.empty()) {
-      conformance->setWitness(req, Witness(witness));
+      trySetWitness(Witness(witness));
       continue;
     }
 
     // Set the witness.
-    conformance->setWitness(req, Witness(witness, witnessSubstitutions,
-                                         syntheticEnv, reqToSyntheticSubs));
+    trySetWitness(Witness(witness, witnessSubstitutions,
+                          syntheticEnv, reqToSyntheticSubs));
   }
   assert(rawIDIter <= rawIDs.end() && "read too much");
 
@@ -4558,6 +4620,20 @@ void ModuleFile::finishNormalConformance(NormalProtocolConformance *conformance,
     conformance->setTypeWitness(typeWitness.first, typeWitness.second.first,
                                 typeWitness.second.second);
   }
+  
+  // Fill in opaque value witnesses if we need to.
+  if (needToFillInOpaqueValueWitnesses) {
+    for (auto member : proto->getMembers()) {
+      // We only care about non-associated-type requirements.
+      auto valueMember = dyn_cast<ValueDecl>(member);
+      if (!valueMember || !valueMember->isProtocolRequirement()
+          || isa<AssociatedTypeDecl>(valueMember))
+        continue;
+      
+      if (!conformance->hasWitness(valueMember))
+        conformance->setWitness(valueMember, Witness::forOpaque(valueMember));
+    }
+  }
 }
 
 GenericEnvironment *ModuleFile::loadGenericEnvironment(const DeclContext *decl,

lib/Serialization/ModuleFile.cpp

@@ -200,7 +200,9 @@ validateControlBlock(llvm::BitstreamCursor &cursor,
       default:
         // Add new cases here, in descending order.
       case 4:
-        result.compatibilityVersion = blobData.substr(scratch[2]+1, scratch[3]);
+        result.compatibilityVersion =
+          version::Version(blobData.substr(scratch[2]+1, scratch[3]),
+                           SourceLoc(), nullptr);
         LLVM_FALLTHROUGH;
       case 3:
         result.shortVersion = blobData.slice(0, scratch[2]);

test/Compatibility/MixAndMatch/Inputs/SomeObjCModule.apinotes

@@ -0,0 +1,9 @@
+Name: SomeObjCModule
+Classes:
+- Name: NSRuncibleSpoon
+  SwiftBridge: RuncibleSpoon
+SwiftVersions:
+- Version: 3
+  Classes:
+  - Name: NSRuncibleSpoon
+    SwiftBridge: ""

test/Compatibility/MixAndMatch/Inputs/SomeObjCModule.h

@@ -0,0 +1,24 @@
+// Swift 3 sees the ObjC class NSRuncibleSpoon as the class, and uses methods
+// with type signatures involving NSRuncibleSpoon to conform to protocols
+// across the language boundary. Swift 4 sees the type as bridged to
+// a RuncibleSpoon value type, but still needs to be able to use conformances
+// declared by Swift 3.
+
+@import Foundation;
+
+@interface NSRuncibleSpoon: NSObject
+@end
+
+@interface SomeObjCClass: NSObject
+- (instancetype _Nonnull)initWithSomeSwiftInitRequirement:(NSRuncibleSpoon* _Nonnull)s;
+- (void)someSwiftMethodRequirement:(NSRuncibleSpoon* _Nonnull)s;
+@property NSRuncibleSpoon * _Nonnull someSwiftPropertyRequirement;
+@end
+
+@protocol SomeObjCProtocol
+- (instancetype _Nonnull)initWithSomeObjCInitRequirement:(NSRuncibleSpoon * _Nonnull)string;
+- (void)someObjCMethodRequirement:(NSRuncibleSpoon * _Nonnull)string;
+@property NSRuncibleSpoon * _Nonnull someObjCPropertyRequirement;
+@end
+
+

test/Compatibility/MixAndMatch/Inputs/SomeObjCModuleX.swift

@@ -0,0 +1,22 @@
+// NB: This file is not named SomeObjCModule.swift to avoid getting picked up
+// by -enable-source-import
+
+@_exported import SomeObjCModule
+
+public struct RuncibleSpoon: _ObjectiveCBridgeable {
+  public init() {}
+
+  public func _bridgeToObjectiveC() -> NSRuncibleSpoon {
+    fatalError()
+  }
+  public static func _forceBridgeFromObjectiveC(_: NSRuncibleSpoon, result: inout RuncibleSpoon?) {
+    fatalError()
+  }
+  public static func _conditionallyBridgeFromObjectiveC(_: NSRuncibleSpoon, result: inout RuncibleSpoon?) -> Bool {
+    fatalError()
+  }
+  public static func _unconditionallyBridgeFromObjectiveC(_: NSRuncibleSpoon?) -> RuncibleSpoon {
+    fatalError()
+  }
+}
+

test/Compatibility/MixAndMatch/Inputs/module.modulemap

@@ -0,0 +1,4 @@
+module SomeObjCModule {
+  header "SomeObjCModule.h"
+  export *
+}

test/Compatibility/MixAndMatch/Inputs/witness_change_swift3_leaf.swift

@@ -0,0 +1,71 @@
+
+// Swift 3 sees the ObjC class NSRuncibleSpoon as the class, and uses methods
+// with type signatures involving NSRuncibleSpoon to conform to protocols
+// across the language boundary. Swift 4 sees the type as bridged to
+// a RuncibleSpoon value type, but still needs to be able to use conformances
+// declared by Swift 3.
+
+// Swift 3, importing Swift 3 and Swift 4 code
+
+import SomeObjCModule
+import SomeSwift3Module
+import SomeSwift4Module
+
+func testMatchAndMix(bridged: RuncibleSpoon, unbridged: NSRuncibleSpoon) {
+  let objcInstanceViaClass
+    = SomeObjCClass(someSwiftInitRequirement: unbridged)
+
+  let objcClassAsS3Protocol: SomeSwift3Protocol.Type = SomeObjCClass.self
+  let objcInstanceViaS3Protocol
+    = objcClassAsS3Protocol.init(someSwiftInitRequirement: unbridged)
+
+  let objcClassAsS4Protocol: SomeSwift4Protocol.Type = SomeObjCClass.self
+  let objcInstanceViaS4Protocol
+    = objcClassAsS4Protocol.init(someSwiftInitRequirement: bridged)
+
+  var bridgedSink: RuncibleSpoon
+  var unbridgedSink: NSRuncibleSpoon
+
+  let swiftPropertyViaClass = objcInstanceViaClass.someSwiftPropertyRequirement
+  unbridgedSink = swiftPropertyViaClass
+  let swiftPropertyViaS3Protocol = objcInstanceViaS3Protocol.someSwiftPropertyRequirement
+  unbridgedSink = swiftPropertyViaS3Protocol
+  let swiftPropertyViaS4Protocol = objcInstanceViaS4Protocol.someSwiftPropertyRequirement
+  bridgedSink = swiftPropertyViaS4Protocol
+
+  objcInstanceViaClass.someSwiftMethodRequirement(unbridged)
+  objcInstanceViaS3Protocol.someSwiftMethodRequirement(unbridged)
+  objcInstanceViaS4Protocol.someSwiftMethodRequirement(bridged)
+
+  let swift3InstanceViaClass
+    = SomeSwift3Class(someObjCInitRequirement: unbridged)
+  let swift3ClassAsProtocol: SomeObjCProtocol.Type = SomeSwift3Class.self
+  let swift3InstanceViaProtocol
+    = swift3ClassAsProtocol.init(someObjCInitRequirement: unbridged)
+  
+  let objcPropertyViaClassS3 = swift3InstanceViaClass.someObjCPropertyRequirement
+  unbridgedSink = objcPropertyViaClassS3
+  let objcPropertyViaProtocolS3 = swift3InstanceViaProtocol.someObjCPropertyRequirement
+  unbridgedSink = objcPropertyViaProtocolS3
+
+  swift3InstanceViaClass.someObjCMethodRequirement(unbridged)
+  swift3InstanceViaProtocol.someObjCMethodRequirement(unbridged)
+
+  let swift4InstanceViaClass
+    = SomeSwift4Class(someObjCInitRequirement: bridged)
+  let swift4ClassAsProtocol: SomeObjCProtocol.Type = SomeSwift4Class.self
+  let swift4InstanceViaProtocol
+    = swift4ClassAsProtocol.init(someObjCInitRequirement: unbridged)
+  
+  let objcPropertyViaClassS4 = swift4InstanceViaClass.someObjCPropertyRequirement
+  bridgedSink = objcPropertyViaClassS4
+  let objcPropertyViaProtocolS4 = swift4InstanceViaProtocol.someObjCPropertyRequirement
+  unbridgedSink = objcPropertyViaProtocolS4
+
+  swift4InstanceViaClass.someObjCMethodRequirement(bridged)
+  swift4InstanceViaProtocol.someObjCMethodRequirement(unbridged)
+
+  _ = bridgedSink
+  _ = unbridgedSink
+}
+