Skip to content

Fix a compatibility suite regression involving the bridging peephole #13137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/SILGen/SILGenBridging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,14 @@ static ManagedValue emitNativeToCBridgedNonoptionalValue(SILGenFunction &SGF,
// The destination type should be AnyObject in this case.
assert(bridgedType->isEqual(SGF.getASTContext().getAnyObjectType()));

// Blocks bridge to id with a cast.
if (auto nativeFnType = dyn_cast<AnyFunctionType>(nativeType)) {
if (nativeFnType->getRepresentation() ==
FunctionTypeRepresentation::Block) {
return SGF.B.createBlockToAnyObject(loc, v, loweredBridgedTy);
}
}

// If the input argument is known to be an existential, save the runtime
// some work by opening it.
if (nativeType->isExistentialType()) {
Expand Down
12 changes: 12 additions & 0 deletions lib/SILGen/SILGenBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,18 @@ ManagedValue SILGenBuilder::createBridgeObjectToRef(SILLocation loc,
return cloner.clone(result);
}

ManagedValue SILGenBuilder::createBlockToAnyObject(SILLocation loc,
ManagedValue v,
SILType destType) {
assert(destType.isAnyObject());
assert(v.getType().is<SILFunctionType>());
assert(v.getType().castTo<SILFunctionType>()->getRepresentation() ==
SILFunctionTypeRepresentation::Block);

// For now, we don't have a better instruction than this.
return createUncheckedRefCast(loc, v, destType);
}

BranchInst *SILGenBuilder::createBranch(SILLocation loc,
SILBasicBlock *targetBlock,
ArrayRef<ManagedValue> args) {
Expand Down
4 changes: 4 additions & 0 deletions lib/SILGen/SILGenBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,10 @@ class SILGenBuilder : public SILBuilder {
ManagedValue createOpenExistentialBoxValue(SILLocation loc,
ManagedValue original, SILType type);

/// Convert a @convention(block) value to AnyObject.
ManagedValue createBlockToAnyObject(SILLocation loc, ManagedValue block,
SILType type);

using SILBuilder::createOptionalSome;
ManagedValue createOptionalSome(SILLocation Loc, ManagedValue Arg);
ManagedValue createManagedOptionalNone(SILLocation Loc, SILType Type);
Expand Down
11 changes: 9 additions & 2 deletions lib/SILGen/SILGenConvert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1261,7 +1261,14 @@ static bool isValueToAnyConversion(CanType from, CanType to) {
}

assert(to->isAny());
return !from->isAnyClassReferenceType();

// Types that we can easily transform into AnyObject:
// - classes and class-bounded archetypes
// - class existentials, even if not pure-@objc
// - @convention(objc) metatypes
// - @convention(block) functions
return !from->isAnyClassReferenceType() &&
!from->isBridgeableObjectType();
}

/// Check whether this conversion is Any??? to AnyObject???. If the result
Expand All @@ -1275,7 +1282,7 @@ static bool isMatchedAnyToAnyObjectConversion(CanType from, CanType to) {
}

if (from->isAny()) {
assert(to->isAnyObject());
assert(to->lookThroughAllAnyOptionalTypes()->isAnyObject());
return true;
}
return false;
Expand Down
8 changes: 8 additions & 0 deletions lib/SILGen/SILGenPoly.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,14 @@ ManagedValue Transform::transform(ManagedValue v,
SGF.getLoweredLoadableType(outputSubstType));
}

// - block to AnyObject conversion
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make sure this doesn't happen on Linux, where blocks do not use Swift-compatible reference counting?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, #13153.

if (outputSubstType->isAnyObject()) {
if (auto inputFnType = dyn_cast<AnyFunctionType>(inputSubstType)) {
if (inputFnType->getRepresentation() == FunctionTypeRepresentation::Block)
return SGF.B.createBlockToAnyObject(Loc, v, loweredResultTy);
}
}

// - existentials
if (outputSubstType->isAnyExistentialType()) {
// We have to re-abstract payload if its a metatype or a function
Expand Down
1 change: 1 addition & 0 deletions test/Inputs/clang-importer-sdk/usr/include/Foundation.h
Original file line number Diff line number Diff line change
Expand Up @@ -1142,3 +1142,4 @@ void install_global_event_handler(_Nullable event_handler handler);
@end

__nullable id returnNullableId(void);
void takeNullableId(__nullable id);
16 changes: 15 additions & 1 deletion test/SILGen/objc_bridging_peephole.swift
Original file line number Diff line number Diff line change
Expand Up @@ -651,11 +651,25 @@ func foo(p: P) {

// rdar://35402853
// Make sure that we don't peephole AnyObject? -> Any? -> AnyObject naively.
// CHECK: sil hidden @_T022objc_bridging_peephole017testOptionalToNonE6BridgeyyF
// CHECK-LABEL: 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'

// CHECK-LABEL: sil hidden @_T022objc_bridging_peephole34testBlockToOptionalAnyObjectBridgeyyyXB5block_tF
func testBlockToOptionalAnyObjectBridge(block: @escaping @convention(block) () -> ()) {
// CHECK: [[T0:%.*]] = begin_borrow {{%.*}} : $@convention(block) () -> ()
// CHECK-NEXT: [[T1:%.*]] = copy_value [[T0]]
// CHECK-NEXT: [[REF:%.*]] = unchecked_ref_cast [[T1]] : $@convention(block) () -> () to $AnyObject
// CHECK-NEXT: [[OPTREF:%.*]] = enum $Optional<AnyObject>, #Optional.some!enumelt.1, [[REF]] : $AnyObject
// CHECK-NEXT: end_borrow [[T0]]
// CHECK-NEXT: // function_ref
// CHECK-NEXT: [[FN:%.*]] = function_ref @takeNullableId : $@convention(c) (Optional<AnyObject>) -> ()
// CHECK-NEXT: apply [[FN]]([[OPTREF]])
// CHECK-NEXT: destroy_value [[OPTREF]]
takeNullableId(block as Any)
}