Skip to content

Commit 549efbc

Browse files
committed
Don't naively apply the bridging peephole to AnyObject? -> Any? -> AnyObject.
Also, fix some logic in the peephole that was incorrectly recognizing T? -> U? -> AnyObject as a force instead of a request to create NSNull. In both cases, we ought to still be able to do much better than the unpeepholed code by just applying the peephole and then merging in NSNull in the nil case, but in the short term we need to fix this bug. Fixes rdar://35402853
1 parent a6796e0 commit 549efbc

File tree

4 files changed

+69
-11
lines changed

4 files changed

+69
-11
lines changed

lib/SILGen/SILGenBridging.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,13 @@ static ManagedValue emitNativeToCBridgedNonoptionalValue(SILGenFunction &SGF,
632632
loweredBridgedTy.castTo<SILFunctionType>());
633633
}
634634

635+
// Erase IUO at this point, because there aren't any conformances for
636+
// IUO anymore. Note that the value representation stays the same
637+
// because SIL erases the difference.
638+
if (auto obj = nativeType->getImplicitlyUnwrappedOptionalObjectType()) {
639+
nativeType = OptionalType::get(obj)->getCanonicalType();
640+
}
641+
635642
// If the native type conforms to _ObjectiveCBridgeable, use its
636643
// _bridgeToObjectiveC witness.
637644
if (auto conformance =

lib/SILGen/SILGenConvert.cpp

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1272,6 +1272,23 @@ static bool isValueToAnyConversion(CanType from, CanType to) {
12721272
return !from->isAnyClassReferenceType();
12731273
}
12741274

1275+
/// Check whether this conversion is Any??? to AnyObject???. If the result
1276+
/// type is less optional, it doesn't count.
1277+
static bool isMatchedAnyToAnyObjectConversion(CanType from, CanType to) {
1278+
while (auto fromObject = from.getAnyOptionalObjectType()) {
1279+
auto toObject = to.getAnyOptionalObjectType();
1280+
if (!toObject) return false;
1281+
from = fromObject;
1282+
to = toObject;
1283+
}
1284+
1285+
if (from->isAny()) {
1286+
assert(to->isAnyObject());
1287+
return true;
1288+
}
1289+
return false;
1290+
}
1291+
12751292
Optional<ConversionPeepholeHint>
12761293
Lowering::canPeepholeConversions(SILGenFunction &SGF,
12771294
const Conversion &outerConversion,
@@ -1332,8 +1349,7 @@ Lowering::canPeepholeConversions(SILGenFunction &SGF,
13321349

13331350
// Converting to Any doesn't do anything semantically special, so we
13341351
// can apply the peephole unconditionally.
1335-
if (intermediateType->lookThroughAllAnyOptionalTypes()->isAny()) {
1336-
assert(resultType->lookThroughAllAnyOptionalTypes()->isAnyObject());
1352+
if (isMatchedAnyToAnyObjectConversion(intermediateType, resultType)) {
13371353
if (loweredSourceTy == loweredResultTy) {
13381354
return applyPeephole(ConversionPeepholeHint::Identity);
13391355
} else if (isValueToAnyConversion(sourceType, intermediateType)) {
@@ -1375,7 +1391,8 @@ Lowering::canPeepholeConversions(SILGenFunction &SGF,
13751391
if (!forced &&
13761392
innerConversion.getKind() == Conversion::BridgeResultFromObjC) {
13771393
if (auto sourceValueType = sourceType.getAnyOptionalObjectType()) {
1378-
if (areRelatedTypesForBridgingPeephole(sourceValueType, resultType)) {
1394+
if (!intermediateType.getAnyOptionalObjectType() &&
1395+
areRelatedTypesForBridgingPeephole(sourceValueType, resultType)) {
13791396
forced = true;
13801397
return applyPeephole(ConversionPeepholeHint::Subtype);
13811398
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1140,3 +1140,5 @@ void install_global_event_handler(_Nullable event_handler handler);
11401140
forKeyPath: (NSString*) keyPath
11411141
context: (void*) options;
11421142
@end
1143+
1144+
__nullable id returnNullableId(void);

test/SILGen/objc_bridging_peephole.swift

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,17 +115,27 @@ func testForcedMethodResult(dummy: DummyClass) {
115115
// CHECK-NEXT: apply [[USE]]([[RESULT]])
116116
useNS(dummy.fetchNullproneString() as NSString)
117117

118+
// This is not a force.
119+
// TODO: we could do it more efficiently than this, though
118120
// CHECK: [[SELF:%.*]] = begin_borrow %0
119121
// CHECK-NEXT: [[METHOD:%.*]] = objc_method
120122
// CHECK-NEXT: [[RESULT:%.*]] = apply [[METHOD]]([[SELF]])
121123
// CHECK-NEXT: end_borrow [[SELF]] from %0
122124
// CHECK-NEXT: switch_enum [[RESULT]]
123-
// CHECK: bb3:
124-
// CHECK: function_ref @_T0s30_diagnoseUnexpectedNilOptionalyBp14_filenameStart_Bw01_E6LengthBi1_01_E7IsASCIIBw5_linetF
125-
// CHECK: bb4([[RESULT:%.*]] : @owned $NSString):
126-
// CHECK-NEXT: [[ANYOBJECT:%.*]] = init_existential_ref [[RESULT]] : $NSString : $NSString, $AnyObject
125+
// CHECK: bb3([[RESULT:%.*]] : @owned $NSString):
126+
// CHECK: function_ref @_T0SS10FoundationE36_unconditionallyBridgeFromObjectiveCSSSo8NSStringCSgFZ
127+
// CHECK: bb4:
128+
// CHECK: enum $Optional<String>, #Optional.none
129+
// CHECK: bb5([[OPTSTRING:%.*]] : @owned $Optional<String>):
130+
// CHECK: [[BRIDGE:%.*]] = function_ref @_T0Sq19_bridgeToObjectiveCyXlyF
131+
// CHECK-NEXT: [[TEMP:%.*]] = alloc_stack $Optional<String>
132+
// CHECK-NEXT: [[BORROW:%.*]] = begin_borrow [[OPTSTRING]]
133+
// CHECK-NEXT: store_borrow [[BORROW]] to [[TEMP]] : $*Optional<String>
134+
// CHECK-NEXT: [[ANYOBJECT:%.*]] = apply [[BRIDGE]]<String>([[TEMP]])
127135
// CHECK: [[USE:%.*]] = function_ref @_T022objc_bridging_peephole12useAnyObjectyyXlF
128136
// CHECK-NEXT: apply [[USE]]([[ANYOBJECT]])
137+
// CHECK-NEXT: end_borrow [[BORROW]] from [[OPTSTRING]]
138+
// CHECK-NEXT: dealloc_stack [[TEMP]]
129139
useAnyObject(dummy.fetchNullproneString() as AnyObject)
130140

131141
// CHECK: return
@@ -233,16 +243,27 @@ func testForcedPropertyValue(dummy: DummyClass) {
233243
// CHECK-NEXT: end_borrow [[SELF]] from %0
234244
useNS(dummy.nullproneStringProperty as NSString)
235245

246+
// This is not a force.
247+
// TODO: we could do it more efficiently than this, though
236248
// CHECK-NEXT: [[SELF:%.*]] = begin_borrow %0
237249
// CHECK-NEXT: [[METHOD:%.*]] = objc_method
238250
// CHECK-NEXT: [[RESULT:%.*]] = apply [[METHOD]]([[SELF]])
239251
// CHECK-NEXT: switch_enum [[RESULT]]
240-
// CHECK: bb3:
241-
// CHECK: function_ref @_T0s30_diagnoseUnexpectedNilOptionalyBp14_filenameStart_Bw01_E6LengthBi1_01_E7IsASCIIBw5_linetF
242-
// CHECK: bb4([[RESULT:%.*]] : @owned $NSString):
243-
// CHECK-NEXT: [[ANYOBJECT:%.*]] = init_existential_ref [[RESULT]] : $NSString : $NSString, $AnyObject
252+
// CHECK: bb3([[RESULT:%.*]] : @owned $NSString):
253+
// CHECK: function_ref @_T0SS10FoundationE36_unconditionallyBridgeFromObjectiveCSSSo8NSStringCSgFZ
254+
// CHECK: bb4:
255+
// CHECK: enum $Optional<String>, #Optional.none
256+
// CHECK: bb5([[OPTSTRING:%.*]] : @owned $Optional<String>):
257+
// CHECK: [[BRIDGE:%.*]] = function_ref @_T0Sq19_bridgeToObjectiveCyXlyF
258+
// CHECK-NEXT: [[TEMP:%.*]] = alloc_stack $Optional<String>
259+
// CHECK-NEXT: [[BORROW:%.*]] = begin_borrow [[OPTSTRING]]
260+
// CHECK-NEXT: store_borrow [[BORROW]] to [[TEMP]] : $*Optional<String>
261+
// CHECK-NEXT: [[ANYOBJECT:%.*]] = apply [[BRIDGE]]<String>([[TEMP]])
244262
// CHECK: [[USE:%.*]] = function_ref @_T022objc_bridging_peephole12useAnyObjectyyXlF
245263
// CHECK-NEXT: apply [[USE]]([[ANYOBJECT]])
264+
// CHECK-NEXT: end_borrow [[BORROW]] from [[OPTSTRING]]
265+
// CHECK-NEXT: dealloc_stack [[TEMP]]
266+
// CHECK-NEXT: destroy_value [[OPTSTRING]]
246267
// CHECK-NEXT: end_borrow [[SELF]] from %0
247268
useAnyObject(dummy.nullproneStringProperty as AnyObject)
248269

@@ -627,3 +648,14 @@ protocol P {
627648
func foo(p: P) {
628649
DummyClass().takeNullableString(p.title)
629650
}
651+
652+
// rdar://35402853
653+
// Make sure that we don't peephole AnyObject? -> Any? -> AnyObject naively.
654+
// CHECK: sil hidden @_T022objc_bridging_peephole017testOptionalToNonE6BridgeyyF
655+
func testOptionalToNonOptionalBridge() {
656+
// CHECK: apply {{.*}}() : $@convention(c) () -> @autoreleased Optional<AnyObject>
657+
// CHECK: function_ref @_T0s018_bridgeAnyObjectToB0ypyXlSgF :
658+
// CHECK: [[T0:%.*]] = function_ref @_T0Sq19_bridgeToObjectiveCyXlyF
659+
// CHECK: apply [[T0]]<Any>
660+
useAnyObject(returnNullableId() as AnyObject)
661+
} // CHECK: end sil function '_T022objc_bridging_peephole017testOptionalToNonE6BridgeyyF'

0 commit comments

Comments
 (0)