From c262248a272cc82f2c2422bb5b62a0e8eca90742 Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Fri, 8 Nov 2024 13:49:21 -0800 Subject: [PATCH] [ObjC Bridging] Consistently bridge block types verbatim A `@convention(block)` closure in Swift is completely compatible with Objective-C and does not need to be wrapped in a `__SwiftValue` box for use. Previously, it was bridged verbatim when appearing by itself, but could end up boxed when it went through array bridging. The test verifies that: * Objective-C does not see a `__SwiftValue` box * Swift `type(of:)` does not see a `__SwiftValue` box * Objective-C can actually call the closure Resolves rdar://138132321 --- stdlib/public/runtime/Casting.cpp | 26 +++++-- test/Casting/Cast_Blocks.swift | 76 +++++++++++++++++++ test/Casting/Inputs/Cast_Blocks/Cast_Blocks.h | 9 +++ test/Casting/Inputs/Cast_Blocks/Cast_Blocks.m | 19 +++++ .../Inputs/Cast_Blocks/module.modulemap | 4 + 5 files changed, 126 insertions(+), 8 deletions(-) create mode 100644 test/Casting/Cast_Blocks.swift create mode 100644 test/Casting/Inputs/Cast_Blocks/Cast_Blocks.h create mode 100644 test/Casting/Inputs/Cast_Blocks/Cast_Blocks.m create mode 100644 test/Casting/Inputs/Cast_Blocks/module.modulemap diff --git a/stdlib/public/runtime/Casting.cpp b/stdlib/public/runtime/Casting.cpp index f375bdce8770c..093c58a189547 100644 --- a/stdlib/public/runtime/Casting.cpp +++ b/stdlib/public/runtime/Casting.cpp @@ -1289,9 +1289,8 @@ static id bridgeAnythingNonVerbatimToObjectiveC(OpaqueValue *src, swift_unknownObjectRetain(result); return result; } - // Dig through existential types. - if (auto srcExistentialTy = dyn_cast(srcType)) { + else if (auto srcExistentialTy = dyn_cast(srcType)) { OpaqueValue *srcInnerValue; const Metadata *srcInnerType; bool isOutOfLine; @@ -1320,10 +1319,9 @@ static id bridgeAnythingNonVerbatimToObjectiveC(OpaqueValue *src, } return result; } - - // Handle metatypes. - if (isa(srcType) - || isa(srcType)) { + // Handle metatypes and existential metatypes + else if (isa(srcType) + || isa(srcType)) { const Metadata *srcMetatypeValue; memcpy(&srcMetatypeValue, src, sizeof(srcMetatypeValue)); @@ -1342,8 +1340,9 @@ static id bridgeAnythingNonVerbatimToObjectiveC(OpaqueValue *src, return objc_retain(protocolObj); } } + } // Handle bridgeable types. - } else if (auto srcBridgeWitness = findBridgeWitness(srcType)) { + else if (auto srcBridgeWitness = findBridgeWitness(srcType)) { // Bridge the source value to an object. auto srcBridgedObject = srcBridgeWitness->bridgeToObjectiveC(src, srcType, srcBridgeWitness); @@ -1353,13 +1352,24 @@ static id bridgeAnythingNonVerbatimToObjectiveC(OpaqueValue *src, srcType->vw_destroy(src); return (id)srcBridgedObject; + } // Handle Errors. - } else if (auto srcErrorWitness = findErrorWitness(srcType)) { + else if (auto srcErrorWitness = findErrorWitness(srcType)) { // Bridge the source value to an NSError. auto flags = consume ? DynamicCastFlags::TakeOnSuccess : DynamicCastFlags::Default; return dynamicCastValueToNSError(src, srcType, srcErrorWitness, flags); } + // Handle functions: "Block" types can be bridged literally + else if (auto fn = dyn_cast(srcType)) { + if (fn->getConvention() == FunctionMetadataConvention::Block) { + id result; + memcpy(&result, src, sizeof(id)); + if (!consume) + swift_unknownObjectRetain(result); + return result; + } + } // Fall back to boxing. return (id)bridgeAnythingToSwiftValueObject(src, srcType, consume); diff --git a/test/Casting/Cast_Blocks.swift b/test/Casting/Cast_Blocks.swift new file mode 100644 index 0000000000000..da1e8956542a0 --- /dev/null +++ b/test/Casting/Cast_Blocks.swift @@ -0,0 +1,76 @@ +// Casts.swift - Tests for conversion between types. +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +// ----------------------------------------------------------------------------- +/// +/// Contains tests for non-trapping type conversions reported by users. +/// +// ----------------------------------------------------------------------------- +// RUN: %empty-directory(%t) +// +// RUN: %clang %target-cc-options -isysroot %sdk -fobjc-arc %S/Inputs/Cast_Blocks/Cast_Blocks.m -c -o %t/Cast_Blocks.o -g +// +// RUN: %target-build-swift -I %S/Inputs/Cast_Blocks %t/Cast_Blocks.o -swift-version 6 -g -Onone -module-name a %s -o %t/a.swift6.Onone.out +// RUN: %target-codesign %t/a.swift6.Onone.out +// RUN: %target-run %t/a.swift6.Onone.out +// +// RUN: %target-build-swift -I %S/Inputs/Cast_Blocks %t/Cast_Blocks.o -swift-version 6 -g -O -module-name a %s -o %t/a.swift6.O.out +// RUN: %target-codesign %t/a.swift6.O.out +// RUN: %target-run %t/a.swift6.O.out +// +// RUN: %target-build-swift -I %S/Inputs/Cast_Blocks %t/Cast_Blocks.o -swift-version 5 -g -Onone -module-name a %s -o %t/a.swift5.Onone.out +// RUN: %target-codesign %t/a.swift5.Onone.out +// RUN: %target-run %t/a.swift5.Onone.out +// +// RUN: %target-build-swift -I %S/Inputs/Cast_Blocks %t/Cast_Blocks.o -swift-version 5 -g -O -module-name a %s -o %t/a.swift5.O.out +// RUN: %target-codesign %t/a.swift5.O.out +// RUN: %target-run %t/a.swift5.O.out +// +// REQUIRES: executable_test +// REQUIRES: objc_interop +// UNSUPPORTED: use_os_stdlib +// UNSUPPORTED: back_deployment_runtime + +import StdlibUnittest +import Foundation +import Cast_Blocks + +fileprivate func SwiftThinksObjectIsSwiftValue(_ t: T) -> Bool { + let type = "\(type(of: t))" + return type == "__SwiftValue" +} + +let CastsTests = TestSuite("Cast_Blocks") + +CastsTests.test("block closures should bridge without __SwiftValue") +{ + let x: @convention(block) () -> Void = {} + + expectFalse(ObjCThinksObjectIsSwiftValue(x)) + + expectFalse(SwiftThinksObjectIsSwiftValue(x)) + + ObjCCanCallBlock(x); +} + +// Bug: @convention(block) closure used to be incorrectly wrapped in +// SwiftValue box when bridged to Objective-C as a member of an array +CastsTests.test("block closures in array should bridge without __SwiftValue") +{ + let f: @convention(block) () -> Void = {} + let x = ([f] as NSArray)[0] + + expectFalse(ObjCThinksObjectIsSwiftValue(x)) + expectFalse(SwiftThinksObjectIsSwiftValue(x)) + ObjCCanCallBlock(x); +} + + +runAllTests() diff --git a/test/Casting/Inputs/Cast_Blocks/Cast_Blocks.h b/test/Casting/Inputs/Cast_Blocks/Cast_Blocks.h new file mode 100644 index 0000000000000..81bd5641411e4 --- /dev/null +++ b/test/Casting/Inputs/Cast_Blocks/Cast_Blocks.h @@ -0,0 +1,9 @@ +#ifndef SWIFT_TEST_CAST_BLOCKS_H +#define SWIFT_TEST_CAST_BLOCKS_H + +#import + +BOOL ObjCThinksObjectIsSwiftValue(id obj); +void ObjCCanCallBlock(id block_as_id); + +#endif diff --git a/test/Casting/Inputs/Cast_Blocks/Cast_Blocks.m b/test/Casting/Inputs/Cast_Blocks/Cast_Blocks.m new file mode 100644 index 0000000000000..58770b6277002 --- /dev/null +++ b/test/Casting/Inputs/Cast_Blocks/Cast_Blocks.m @@ -0,0 +1,19 @@ +#import + +#import "Cast_Blocks.h" + +BOOL ObjCThinksObjectIsSwiftValue(id obj) { + Class cls = object_getClass(obj); + const char *name = class_getName(cls); + if (strcmp(name, "__SwiftValue") == 0) { + return TRUE; + } else { + return FALSE; + } +} + +void ObjCCanCallBlock(id block_as_id) { + typedef void(^blockType)(void); + blockType block = (blockType)block_as_id; + block(); +} diff --git a/test/Casting/Inputs/Cast_Blocks/module.modulemap b/test/Casting/Inputs/Cast_Blocks/module.modulemap new file mode 100644 index 0000000000000..0ca4c59b228d3 --- /dev/null +++ b/test/Casting/Inputs/Cast_Blocks/module.modulemap @@ -0,0 +1,4 @@ +module Cast_Blocks { + header "Cast_Blocks.h" + export * +}