Merge pull request #9403 from jckarter/mix-and-match-witnesses-4.0

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

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
+}
+