[stdlib] Conform fixed-width integer types to LosslessStringConvertible #11193
Conversation
| /// | ||
| /// - Parameter description: The ASCII representation of a number in base 10. | ||
| @_semantics("optimize.sil.specialize.generic.partial.never") | ||
| @inline(__always) |
There was a problem hiding this comment.
Are these appropriate and optimal uses of these internal attributes? I'm using the example of other initializers implemented in this file and Integers.swift.gyb.
There was a problem hiding this comment.
/cc @swiftix for the usage of the @_semantics attribute.
@inline(__always) is fine.
There was a problem hiding this comment.
/cc @natecook1000 for the doc comment.
There was a problem hiding this comment.
Can you include an out of range example? e.g.:
Int("10000000000000000000000000") // Out of range
You could remove "in base 10" from the last paragraph, too.
|
I'm unable to test this locally, as the |
|
+1 This PR fixes: SR-5558 |
|
@swift-ci Please smoke test.
The work-around some of us are using is commenting out the contents of ModelIO.swift, since nothing uses it. |
|
@swift-ci Please smoke test |
|
@swift-ci Please Test Source Compatibility |
|
@xwu Thanks!
Is there a reason you haven't implemented a test as part of this PR? |
moiseev
changed the title
| /// | ||
| /// - Parameter description: The ASCII representation of a number in base 10. | ||
| @_semantics("optimize.sil.specialize.generic.partial.never") | ||
| @inline(__always) |
There was a problem hiding this comment.
Can you include an out of range example? e.g.:
Int("10000000000000000000000000") // Out of range
You could remove "in base 10" from the last paragraph, too.
| @@ -102,7 +102,7 @@ extension FixedWidthInteger { | |||
| /// | |||
| /// The string passed as `text` may begin with a plus or minus sign character | |||
| /// (`+` or `-`), followed by one or more numeric digits (`0-9`) or letters | |||
| /// (`a-z` or `A-Z`). The string is case insensitive. | |||
| /// (`a-z` or `A-Z`). The string is case-insensitive. | |||
There was a problem hiding this comment.
This is correct as is. "The case-insensitive string…" vs "The string is case insensitive."
There was a problem hiding this comment.
Okay, really the string is just a string. 😅 Can you change this to "The string parsing is case insensitive"?
There was a problem hiding this comment.
@natecook1000 You're right! My personal style is to hyphenate most compound adjectives (with the most common exception arising when the first word is an adverb that ends in "-ly"). However, Chicago Manual rules (which, I believe, is or was at one point Apple's house style) calls for the spelling "case insensitive" when it follows the noun.
Also an astute point about what exactly is case-insensitive. I'd tweak it a little and write: "Parsing of the string is case insensitive."
|
Somewhat related to #11157 |
|
@moiseev Yes, there was a very good reason: it was bedtime last night and I had fifteen hours of patient care duties today :) But now I've added one. |
test/stdlib/Integers.swift.gyb
Outdated
| } | ||
|
|
||
| let i: [Int] = _toArray("1 2 3") | ||
| expectEqual(i.count, 3) |
There was a problem hiding this comment.
FYI: there exists 'expectEqualSequence` https://github.com/apple/swift/blob/master/stdlib/private/StdlibUnittest/StdlibUnittest.swift.gyb#L2326.
So one can write checks like these in one shot:
expectEqualSequence([1, 2, 3], _toArray("1 2 3"))
|
@swift-ci Please test |
|
Build failed |
|
@moiseev Sorry, let's try this again; a minor change was necessary. |
|
Build failed |
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
|
@swift-ci Please Test Source Compatibility |
As approved in SE-0089,
Intand its friends should conform toLosslessStringConvertible.Since
init?(_:radix:)is now implemented onFixedWidthInteger, this PR adopts the following design:FixedWidthIntegernow refinesLosslessStringConvertibleinit?(_:)has a default implementation that callsinit?(_:radix:).There's clearly a test that can be added to test this conformance. What's going to be even more critical is assuring source compatibility.