-
Notifications
You must be signed in to change notification settings - Fork 1.2k
SR-8407: NSNumber as? Bool should only work for 0 or 1 #1642
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
Conversation
- NSNumber as? Bool should only work for NSNumber values 0 and 1 to match Darwin.
@swift-ci please test |
I recall @phausler doing a lot of work in this area last year as far as bridging goes; do we need to update it elsewhere here now that we have more consistent bridging with linux? |
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.
We should follow up with a correction of the Darwin compat for 32 bit truncation on boolValue.
|
||
XCTAssertEqual(NSNumber(value: Int64.max).boolValue, true) | ||
XCTAssertEqual(NSNumber(value: Int64.max - 1).boolValue, true) | ||
XCTAssertEqual(NSNumber(value: Int64.min).boolValue, false) // Darwin compatibility |
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 no longer the case, we finally fixed that in High Sierra.
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.
Are you sure? I tested on HS 10.13.6:
print(NSNumber(value: Int64.min).boolValue)
false
|
||
XCTAssertEqual(NSNumber(value: Int.max).boolValue, true) | ||
XCTAssertEqual(NSNumber(value: Int.max - 1).boolValue, true) | ||
XCTAssertEqual(NSNumber(value: Int.min).boolValue, false) // Darwin compatibility |
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.
ditto here
@@ -623,6 +624,37 @@ class TestNSNumberBridging : XCTestCase { | |||
XCTAssertEqual(value, ns_value) | |||
} | |||
} | |||
|
|||
func testNSNumberToBool() { |
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 correct behavior, thanks for the update!
@parkera The behaviour for |
Ok, we can do that. |
match Darwin.