swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.58k stars 10.36k forks source link

[SR-11397] FixedWidthInteger.init?(_: radix:) fails for Self with short bitWidth #53798

Open swift-ci opened 5 years ago

swift-ci commented 5 years ago
Previous ID SR-11397
Radar None
Original Reporter CTMacUser (JIRA User)
Type Bug
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Standard Library | |Labels | Bug, StarterBug | |Assignee | @theblixguy | |Priority | Medium | md5: 71036ce9352bad83dc05a0cfcdea3df7

Issue Description:

The current implementation of FixedWidthInteger.init?(_: radix: ) converts the radix to the receiver type. If that type has less than 6 bits of width, not all radix values are supported and a run error happens at the conversion.

Theoretically, any width above zero should work if we go to a bignum implementation. I'm not suggesting we do that, but that workaround indicates we shouldn't return a nil from the initializer in such cases, because that would imply a user error and not a implementation error. I guess a fatal error is more appropriate.

(BTW, my code was testing with a zero-width type, so there may be other problems.)

stephentyrone commented 5 years ago

This should be pretty easy to fix for someone interested in this aspect of the stdlib. It's mostly just a matter of replumbing `_parseASCII` to take the `radix` parameter as an `Int` instead of `Result`. This would actually be a nice performance optimization in some cases anyway.

Good starter bug.

theblixguy commented 5 years ago

PR: https://github.com/apple/swift/pull/26961

swift-ci commented 4 years ago

Comment by Andreas Jönsson (JIRA)

If this one is still open I'd like to give it a try @stephentyrone, @theblixguy?

theblixguy commented 4 years ago

I haven’t found the time to address the review comment on my PR, but I’ll be doing that soon. Perhaps you can look for another StarterBug?

swift-ci commented 4 years ago

Comment by Andreas Jönsson (JIRA)

Sure thing! 🙂 And thanks for the quick reply.