Merge pull request #13078 from rjmccall/bridging-peephole-optionality-bug

Don't naively apply the bridging peephole to AnyObject? -> Any? -> AnyObject

Author: rjmccall

lib/SILGen/SILGenBridging.cpp

@@ -631,6 +631,13 @@ static ManagedValue emitNativeToCBridgedNonoptionalValue(SILGenFunction &SGF,
                                  loweredBridgedTy.castTo<SILFunctionType>());
   }
 
+  // Erase IUO at this point, because there aren't any conformances for
+  // IUO anymore.  Note that the value representation stays the same
+  // because SIL erases the difference.
+  if (auto obj = nativeType->getImplicitlyUnwrappedOptionalObjectType()) {
+    nativeType = OptionalType::get(obj)->getCanonicalType();
+  }
+
   // If the native type conforms to _ObjectiveCBridgeable, use its
   // _bridgeToObjectiveC witness.
   if (auto conformance =

lib/SILGen/SILGenConvert.cpp

@@ -1272,6 +1272,23 @@ static bool isValueToAnyConversion(CanType from, CanType to) {
   return !from->isAnyClassReferenceType();
 }
 
+/// Check whether this conversion is Any??? to AnyObject???.  If the result
+/// type is less optional, it doesn't count.
+static bool isMatchedAnyToAnyObjectConversion(CanType from, CanType to) {
+  while (auto fromObject = from.getAnyOptionalObjectType()) {
+    auto toObject = to.getAnyOptionalObjectType();
+    if (!toObject) return false;
+    from = fromObject;
+    to = toObject;
+  }
+
+  if (from->isAny()) {
+    assert(to->isAnyObject());
+    return true;
+  }
+  return false;
+}
+
 Optional<ConversionPeepholeHint>
 Lowering::canPeepholeConversions(SILGenFunction &SGF,
                                  const Conversion &outerConversion,
@@ -1332,8 +1349,7 @@ Lowering::canPeepholeConversions(SILGenFunction &SGF,
 
       // Converting to Any doesn't do anything semantically special, so we
       // can apply the peephole unconditionally.
-      if (intermediateType->lookThroughAllAnyOptionalTypes()->isAny()) {
-        assert(resultType->lookThroughAllAnyOptionalTypes()->isAnyObject());
+      if (isMatchedAnyToAnyObjectConversion(intermediateType, resultType)) {
         if (loweredSourceTy == loweredResultTy) {
           return applyPeephole(ConversionPeepholeHint::Identity);
         } else if (isValueToAnyConversion(sourceType, intermediateType)) {
@@ -1375,7 +1391,8 @@ Lowering::canPeepholeConversions(SILGenFunction &SGF,
       if (!forced &&
           innerConversion.getKind() == Conversion::BridgeResultFromObjC) {
         if (auto sourceValueType = sourceType.getAnyOptionalObjectType()) {
-          if (areRelatedTypesForBridgingPeephole(sourceValueType, resultType)) {
+          if (!intermediateType.getAnyOptionalObjectType() &&
+              areRelatedTypesForBridgingPeephole(sourceValueType, resultType)) {
             forced = true;
             return applyPeephole(ConversionPeepholeHint::Subtype);
           }

test/Inputs/clang-importer-sdk/usr/include/Foundation.h

@@ -1140,3 +1140,5 @@ void install_global_event_handler(_Nullable event_handler handler);
          forKeyPath: (NSString*) keyPath
          context: (void*) options;
 @end
+
+__nullable id returnNullableId(void);

test/SILGen/objc_bridging_peephole.swift

@@ -115,17 +115,27 @@ func testForcedMethodResult(dummy: DummyClass) {
   // CHECK-NEXT: apply [[USE]]([[RESULT]])
   useNS(dummy.fetchNullproneString() as NSString)
 
+  //   This is not a force.
+  //   TODO: we could do it more efficiently than this, though
   // CHECK:      [[SELF:%.*]] = begin_borrow %0
   // CHECK-NEXT: [[METHOD:%.*]] = objc_method
   // CHECK-NEXT: [[RESULT:%.*]] = apply [[METHOD]]([[SELF]])
   // CHECK-NEXT: end_borrow [[SELF]] from %0
   // CHECK-NEXT: switch_enum [[RESULT]]
-  // CHECK:    bb3:
-  // CHECK:      function_ref @_T0s30_diagnoseUnexpectedNilOptionalyBp14_filenameStart_Bw01_E6LengthBi1_01_E7IsASCIIBw5_linetF
-  // CHECK:    bb4([[RESULT:%.*]] : @owned $NSString):
-  // CHECK-NEXT: [[ANYOBJECT:%.*]] = init_existential_ref [[RESULT]] : $NSString : $NSString, $AnyObject
+  // CHECK:    bb3([[RESULT:%.*]] : @owned $NSString):
+  // CHECK:      function_ref @_T0SS10FoundationE36_unconditionallyBridgeFromObjectiveCSSSo8NSStringCSgFZ
+  // CHECK:    bb4:
+  // CHECK:      enum $Optional<String>, #Optional.none
+  // CHECK:    bb5([[OPTSTRING:%.*]] : @owned $Optional<String>):
+  // CHECK:      [[BRIDGE:%.*]] = function_ref @_T0Sq19_bridgeToObjectiveCyXlyF
+  // CHECK-NEXT: [[TEMP:%.*]] = alloc_stack $Optional<String>
+  // CHECK-NEXT: [[BORROW:%.*]] = begin_borrow [[OPTSTRING]]
+  // CHECK-NEXT: store_borrow [[BORROW]] to [[TEMP]] : $*Optional<String>
+  // CHECK-NEXT: [[ANYOBJECT:%.*]] = apply [[BRIDGE]]<String>([[TEMP]])
   // CHECK:      [[USE:%.*]] = function_ref @_T022objc_bridging_peephole12useAnyObjectyyXlF
   // CHECK-NEXT: apply [[USE]]([[ANYOBJECT]])
+  // CHECK-NEXT: end_borrow [[BORROW]] from [[OPTSTRING]]
+  // CHECK-NEXT: dealloc_stack [[TEMP]]
   useAnyObject(dummy.fetchNullproneString() as AnyObject)
 
   // CHECK:      return
@@ -233,16 +243,27 @@ func testForcedPropertyValue(dummy: DummyClass) {
   // CHECK-NEXT: end_borrow [[SELF]] from %0
   useNS(dummy.nullproneStringProperty as NSString)
 
+  //   This is not a force.
+  //   TODO: we could do it more efficiently than this, though
   // CHECK-NEXT: [[SELF:%.*]] = begin_borrow %0
   // CHECK-NEXT: [[METHOD:%.*]] = objc_method
   // CHECK-NEXT: [[RESULT:%.*]] = apply [[METHOD]]([[SELF]])
   // CHECK-NEXT: switch_enum [[RESULT]]
-  // CHECK:    bb3:
-  // CHECK:      function_ref @_T0s30_diagnoseUnexpectedNilOptionalyBp14_filenameStart_Bw01_E6LengthBi1_01_E7IsASCIIBw5_linetF
-  // CHECK:    bb4([[RESULT:%.*]] : @owned $NSString):
-  // CHECK-NEXT: [[ANYOBJECT:%.*]] = init_existential_ref [[RESULT]] : $NSString : $NSString, $AnyObject
+  // CHECK:    bb3([[RESULT:%.*]] : @owned $NSString):
+  // CHECK:      function_ref @_T0SS10FoundationE36_unconditionallyBridgeFromObjectiveCSSSo8NSStringCSgFZ
+  // CHECK:    bb4:
+  // CHECK:      enum $Optional<String>, #Optional.none
+  // CHECK:    bb5([[OPTSTRING:%.*]] : @owned $Optional<String>):
+  // CHECK:      [[BRIDGE:%.*]] = function_ref @_T0Sq19_bridgeToObjectiveCyXlyF
+  // CHECK-NEXT: [[TEMP:%.*]] = alloc_stack $Optional<String>
+  // CHECK-NEXT: [[BORROW:%.*]] = begin_borrow [[OPTSTRING]]
+  // CHECK-NEXT: store_borrow [[BORROW]] to [[TEMP]] : $*Optional<String>
+  // CHECK-NEXT: [[ANYOBJECT:%.*]] = apply [[BRIDGE]]<String>([[TEMP]])
   // CHECK:      [[USE:%.*]] = function_ref @_T022objc_bridging_peephole12useAnyObjectyyXlF
   // CHECK-NEXT: apply [[USE]]([[ANYOBJECT]])
+  // CHECK-NEXT: end_borrow [[BORROW]] from [[OPTSTRING]]
+  // CHECK-NEXT: dealloc_stack [[TEMP]]
+  // CHECK-NEXT: destroy_value [[OPTSTRING]]
   // CHECK-NEXT: end_borrow [[SELF]] from %0
   useAnyObject(dummy.nullproneStringProperty as AnyObject)
 
@@ -627,3 +648,14 @@ protocol P {
 func foo(p: P) {
   DummyClass().takeNullableString(p.title)
 }
+
+// rdar://35402853
+//   Make sure that we don't peephole AnyObject? -> Any? -> AnyObject naively.
+// CHECK: sil hidden @_T022objc_bridging_peephole017testOptionalToNonE6BridgeyyF
+func testOptionalToNonOptionalBridge() {
+  // CHECK: apply {{.*}}() : $@convention(c) () -> @autoreleased Optional<AnyObject>
+  // CHECK: function_ref @_T0s018_bridgeAnyObjectToB0ypyXlSgF :
+  // CHECK: [[T0:%.*]] = function_ref @_T0Sq19_bridgeToObjectiveCyXlyF
+  // CHECK: apply [[T0]]<Any>
+  useAnyObject(returnNullableId() as AnyObject)
+} // CHECK: end sil function '_T022objc_bridging_peephole017testOptionalToNonE6BridgeyyF'