NSNumber: update API for SE-0170 #1204
Conversation
|
@swift-ci please test |
| self = value | ||
| } | ||
|
|
||
| public func _bridgeToObjectiveC() -> NSNumber { |
There was a problem hiding this comment.
Question: should this (and other _bridgeToObjectiveC() functions) be marked @_semantics("convertToObjectiveC")?
There was a problem hiding this comment.
For swift corelibs it is fine to omit, iirc that is just a hint not a strict enforcement
| @discardableResult | ||
| static public func _conditionallyBridgeFromObjectiveC(_ source: _ObjectType, result: inout Int?) -> Bool { | ||
| result = source.intValue | ||
| public static func _conditionallyBridgeFromObjectiveC(_ x: NSNumber, result: inout Int?) -> Bool { |
There was a problem hiding this comment.
I removed the @discardableResult annotation as this is not present in the overlay. Is that ok?
There was a problem hiding this comment.
I am not sure what that does for compiler methods
There was a problem hiding this comment.
I guess it generates a warning, I couldn't see why ignoring that would ever be a good idea though.
| self = number.boolValue | ||
| } | ||
|
|
||
| public init?(exactly number: NSNumber) { |
There was a problem hiding this comment.
I'd welcome a close review of this function @phausler.
Foundation/Stream.swift
Outdated
| @@ -119,7 +119,7 @@ open class InputStream: Stream { | |||
|
|
|||
| // reads up to length bytes into the supplied buffer, which must be at least of size len. Returns the actual number of bytes read. | |||
| open func read(_ buffer: UnsafeMutablePointer<UInt8>, maxLength len: Int) -> Int { | |||
| return CFReadStreamRead(_stream, buffer, CFIndex(len._bridgeToObjectiveC())) | |||
| return CFReadStreamRead(_stream, buffer, CFIndex(truncating: len._bridgeToObjectiveC())) | |||
There was a problem hiding this comment.
This is produced by the Xcode fixit. Could it just be return CFReadStreamRead(_stream, buffer, len) though?
There was a problem hiding this comment.
If len is an Int and CFIndex is defined as public typealias CFIndex = Int why is there any bridging at all?
There was a problem hiding this comment.
That's what I wondered!
There was a problem hiding this comment.
Yea let's drop the extra round trip bridge here
Foundation/Stream.swift
Outdated
| @@ -119,7 +119,7 @@ open class InputStream: Stream { | |||
|
|
|||
| // reads up to length bytes into the supplied buffer, which must be at least of size len. Returns the actual number of bytes read. | |||
| open func read(_ buffer: UnsafeMutablePointer<UInt8>, maxLength len: Int) -> Int { | |||
| return CFReadStreamRead(_stream, buffer, CFIndex(len._bridgeToObjectiveC())) | |||
| return CFReadStreamRead(_stream, buffer, CFIndex(truncating: len._bridgeToObjectiveC())) | |||
There was a problem hiding this comment.
Yea let's drop the extra round trip bridge here
| self = number.boolValue | ||
| } | ||
|
|
||
| public init?(exactly number: NSNumber) { |
| @discardableResult | ||
| static public func _conditionallyBridgeFromObjectiveC(_ source: _ObjectType, result: inout Int?) -> Bool { | ||
| result = source.intValue | ||
| public static func _conditionallyBridgeFromObjectiveC(_ x: NSNumber, result: inout Int?) -> Bool { |
There was a problem hiding this comment.
I am not sure what that does for compiler methods
| self = value | ||
| } | ||
|
|
||
| public func _bridgeToObjectiveC() -> NSNumber { |
There was a problem hiding this comment.
For swift corelibs it is fine to omit, iirc that is just a hint not a strict enforcement
| @@ -32,158 +32,523 @@ internal let kCFNumberSInt128Type = CFNumberType(rawValue: 17)! | |||
| internal let kCFNumberSInt128Type: CFNumberType = 17 | |||
| #endif | |||
|
|
|||
| extension Int8 : _ObjectTypeBridgeable { | |||
There was a problem hiding this comment.
This looks like a faithful copy of the overlay, how much effort do you think will be required to make this shared tween the overlay and SCLF?
There was a problem hiding this comment.
The protocol is called _ObjectiveCBridgeable in the overlay - giving it the same name here would be a start. The files will never be identical because SCLF has the class NSNumber definition which isn't in the overlay. But I think we can get them closer.
ianpartridge
force-pushed
the
nsnumber-se-0170
branch
from
7f2c900 to
38b745f
Compare
|
This should be merged after #1213. |
|
@swift-ci please test |
|
@swift-ci please test and merge |
Int(truncating: NSNumber)is missing in swift-corelibs-foundation. This function was added as part of SE-0170.This PR implements the
init(truncating:)andinit?(exactly:)APIs from SE-0170, and also deprecatesinit(_:)appropriately.It seems we also lacked implementations of
init(_:)for many of the primitive number types (e.g.Int8) to begin with so this PR also adds those.