-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix NSNumber.objCType
crashes, make it returns same values with darwin platform version
#1025
Conversation
Expected results are sampled from macOS
fixes SR-4907
…sing `_Factory` protocol
…SNumber` is Boolean
…edicate.test_NSCoding()`
…itectures - Results of `NSNumber.objCType.pointee` depends on architecture if `NSNumber` is initialized with `Int` or `UInt` - When `NSNumber` is initialized with 64-bit integer, The value of `NSNumber.objCType.pointee` switches from 'q' to 'Q' with `Int64.max` as the boundary.
Reflected differences of |
@swift-ci please test |
@@ -78,7 +78,7 @@ internal class __NSCFBoolean : NSNumber { | |||
override var objCType: UnsafePointer<Int8> { | |||
// This must never be fixed to be "B", although that would | |||
// cause correct old-style archiving when this is unarchived. | |||
return UnsafePointer<Int8>(bitPattern: UInt(_NSSimpleObjCType.Char.rawValue.value))! | |||
return String(_NSSimpleObjCType.Char.rawValue)._bridgeToObjectiveC().utf8String! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is going to cause leaks, can we have a static allocation somewhere stashed with the type to cstrings?
|
||
extension _Factory { | ||
init(factory: () -> Self) { | ||
self = factory() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm this is rather strange, if this works perhaps we should investigate using this pattern further
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, just prototyped a test for this... wow! this will solve a ton of issues with swift-corelibs-foundation!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -154,3 +154,14 @@ open class NSValue : NSObject, NSCopying, NSSecureCoding, NSCoding { | |||
} | |||
} | |||
|
|||
extension NSValue : _Factory {} | |||
|
|||
protocol _Factory { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just for maintainability; can we annotate this as internal?
Superseded by #1093 |
Fixes SR-4907
TestNSNumber.test_objCType()
reproducing crashExpected results are sampled from macOS
objCType
property ofNSNumber
Change
NSNumber.objCType
to returning same values with darwin platform versionNSNumber.objCType
to returning same values with darwinNSNumber.init(value: Bool)
to returning__NSCFBoolean
by using_Factory
protocolSince
NSNumber.objCType
returnsc
with both ofBool
andInt8
on darwin platform,NSNumber.init(value: Bool)
is changed to returns__NSCFBoolean
by using_Factory
protocol, and it changed to use_cfTypeID
to distinguish between Bool and Int8.JSONSerialization
to use_cfTypeID
for checking whetherNSNumber
is Boolean or notNSNumber.compare(_:)
to useobjCType
TestNumber.allTests
NSKeyedUnarchiver.decodeBool(forKey:)
and add test toTestNSPredicate.test_NSCoding()