Skip to content

Conversation

@spevans
Copy link
Contributor

@spevans spevans commented Feb 17, 2018

  • If an NSNumber was initialised from a boolean then dont allow it
    to be converted to a numeric value.

Tested against native Darwin Foundation.

- If an NSNumber was initialised from a boolean then dont allow it
  to be converted to a numeric value.
@spevans
Copy link
Contributor Author

spevans commented Feb 17, 2018

@swift-ci please test

@spevans spevans requested a review from itaiferber February 17, 2018 20:48
@spevans
Copy link
Contributor Author

spevans commented Feb 19, 2018

@swift-ci please test

Copy link
Contributor

@itaiferber itaiferber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together! The approach is sound, but do you think we'd be able to get the source changes here closer to what was done in swiftlang/swift#11885? The less our implementations and tests diverge between the overlay and swift-corelibs-foundation, the easier it'll be to to apply similar changes between them.

@spevans
Copy link
Contributor Author

spevans commented Feb 19, 2018

@itaiferber Ive updated it to match the overlay.

@spevans
Copy link
Contributor Author

spevans commented Feb 19, 2018

@swift-ci please test

3 similar comments
@spevans
Copy link
Contributor Author

spevans commented Feb 19, 2018

@swift-ci please test

@alblue
Copy link
Contributor

alblue commented Feb 20, 2018

@swift-ci please test

@spevans
Copy link
Contributor Author

spevans commented Feb 21, 2018

@swift-ci please test

@alblue
Copy link
Contributor

alblue commented Feb 22, 2018

@itaiferber can you re-review the changes after Simon added the subsequent commit? It looks like the tests pass now.

@itaiferber
Copy link
Contributor

@alblue Yes, sorry — this fell off my Radar.

@alblue
Copy link
Contributor

alblue commented Feb 23, 2018

@swift-ci please test and merge

@swift-ci swift-ci merged commit aab4b8a into swiftlang:master Feb 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants