SE-0139: Bridge all standard number types to NSNumber. #4933

Merged
merged 1 commit into from Sep 23, 2016

Projects

None yet

4 participants

@jckarter
Member

Extend NSNumber bridging to cover not only Int, UInt, Double, and Bool, but all of the standard types as well. Extend the TypePreservingNSNumber subclass to accommodate all of these types, so that we preserve type identity for AnyHashable and dynamic casting of Swift-bridged NSNumbers. If a pure Cocoa NSNumber is cast, just trust that the user knows what they're doing.

@jckarter
Member

@swift-ci Please test

@jckarter
Member

@DougGregor Mind reviewing this for inclusion in the Swift 3 branch?

@swift-ci
Contributor

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 3bf86ce
Test requested by - @jckarter

@implementation _SwiftTypePreservingNSNumber {
+ /*alignas(StorageSize)*/ char storage[StorageSize];
@DougGregor
DougGregor Sep 22, 2016 Member

Seems like you want alignas(Double) alignas(int64_t) alignas(uint64_t) to be really picky here.

@jckarter
jckarter Sep 23, 2016 edited Member

Unfortunately, Clang doesn't seem to let you put any alignas decorations at all in an ObjC++ implementation without an "attributes can't be put here" error. It shouldn't really be necessary since we consistently memcpy everything in and out of the storage.

DEFINE_ACCESSOR(int, intValue)
DEFINE_ACCESSOR(unsigned int, unsignedIntValue)
DEFINE_ACCESSOR(long long, longLongValue)
DEFINE_ACCESSOR(unsigned long long, unsignedLongLongValue)
DEFINE_ACCESSOR(float, floatValue)
DEFINE_ACCESSOR(double, doubleValue)
+DEFINE_ACCESSOR(NSInteger, integerValue)
+DEFINE_ACCESSOR(NSUInteger, unsignedIntegerValue)
@DougGregor
DougGregor Sep 22, 2016 Member

Huh, we just forgot these?

@jckarter
jckarter Sep 23, 2016 Member

Yep, and the default implementations in the NSNumber root class just infinitely looped trying to redispatch themselves. Fun.

+nsNumberBridging.test("${AType} to ${BType} casting fails") {
+ let original: ${AType} = 0
+ let bridged = original as AnyObject as Any
+ expectEqual(.none, bridged as? ${BType})
@DougGregor
DougGregor Sep 22, 2016 Member

expectNotNil works for this, FWIW

- }
+ // If the NSNumber instance preserved its Swift type, we only want to allow
+ // the cast if the type matches.
+ if let tag = _SwiftTypePreservingNSNumberTag(rawValue:
@DougGregor
DougGregor Sep 22, 2016 Member

I get the sneaky trick with the NonSwift value here, but it's really subtle. Could you document _SwiftTypePreservingNSNumberTag and it's C counterpart so nobody surprises themselves by trying to put NonSwift in _SwiftTypePreservingNSNumberTag?

@jckarter
jckarter Sep 23, 2016 Member

I elaborated the comment a bit. How's it look now?

@jckarter
Member

@swift-ci Please test

@swift-ci
Contributor

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 3bf86ce
Test requested by - @jckarter

+ /// enums. For an Optional<_SwiftTypePreservingNSNumberTag>, Swift uses the
+ /// first invalid integer value to represent `.none`. This representation on
+ /// the C side allows _SwiftTypePreservingNSNumberTag(rawValue:) on the Swift
+ /// side to compile down to a no-op.
@jrose-apple
jrose-apple Sep 23, 2016 Member

How about using 0 for this instead of 14?

@jckarter
jckarter Sep 23, 2016 Member

That's how it was before, but that then forces a representation change when we convert from the raw C value to the Swift enum on the Swift side, since the Swift enum always uses the compacted representation starting from zero. Definitely a microoptimization.

@jrose-apple
jrose-apple Sep 23, 2016 Member

Oh, bleah, right—the raw value isn't the representation value.

@jckarter jckarter SE-0139: Bridge all standard number types to NSNumber.
Extend NSNumber bridging to cover not only `Int`, `UInt`, `Double`, and `Bool`, but all of the standard types as well. Extend the `TypePreservingNSNumber` subclass to accommodate all of these types, so that we preserve type identity for `AnyHashable` and dynamic casting of Swift-bridged NSNumbers. If a pure Cocoa NSNumber is cast, just trust that the user knows what they're doing.

This XFAILs a couple of serialization tests that attempt to build the Foundation overlay, but which don't properly handle `gyb` files.
9b1f238
@jckarter
Member

@swift-ci Please test

@jckarter jckarter merged commit a506af0 into apple:master Sep 23, 2016

4 checks passed

Swift Test Linux Platform Build finished.
Details
Swift Test Linux Platform (smoke test)
Details
Swift Test OS X Platform Build finished. 43510 tests run, 0 skipped, 0 failed.
Details
Swift Test OS X Platform (smoke test)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment