-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[stdlib] Fix Int(_:radix:) accepting unintentional cases (SR-187) #613
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
@PatrickPijnappel Thank you, this LGTM, but please re-format to 80 columns. |
guard let result = _parseUnsignedAsciiAsUIntMax(digitsUTF16, radix, maximum) else { return nil } | ||
// Disallow < 0 | ||
if hasMinus && result != 0 { return nil } | ||
// Return |
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.
I don't think // Return
is a useful comment :)
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.
The comments are intended as headings for code grouping (instead of newlines) so you can quickly skim the algorithm and it's structure/flow just from those headings. But I agree // Return
in itself it not a very useful comment :). I'll can replace it by a slightly more useful heading, e.g. // Valid result.
(to indicate the algorithm early-returned all invalids), or remove it (/replace it by a newline) if you prefer ;).
Added terminating periods and reformatted to fit 80 columns. |
[stdlib] Fix Int(_:radix:) accepting unintentional cases (SR-187)
…ed out hashes for each repo. I think update-checkout is growing to the point, we should probably rename it to something like repo-tool. The output of this command looks as follows: clang 973bd1a Merge remote-tracking branch 'origin/swift-3.1-branch' into stable cmark 5af77f3 Merge pull request swiftlang#95 from kainjow/master compiler-rt 1f24bd0 Merge remote-tracking branch 'origin/swift-3.1-branch' into stable llbuild c324ee3 Merge pull request swiftlang#35 from tinysun212/pr-cygwin-1 lldb f6a5830 Adjust LLDB for changes to the layout of _SwiftTypePreservingNSNumber llvm 52482d0 Merge remote-tracking branch 'origin/swift-3.1-branch' into stable swift 45f3d2a [update-checkout] Add a small tool to dump hashes for all of the checkout repos. swift-corelibs-foundation cc5985e Loopback tests for URLSession (swiftlang#613) swift-corelibs-libdispatch ba7802e Merge pull request swiftlang#178 from dgrove-oss/depend-on-swiftc swift-corelibs-xctest 51b419d Merge pull request swiftlang#174 from modocache/sr-1901-remove-workarounds swift-integration-tests c95c832 Merge pull request swiftlang#12 from abertelrud/fix-swift-package-init-lib-test swift-xcode-playground-support 4b40c34 Merge pull request swiftlang#10 from apple/stdlib-unittest-expect-nil swiftpm 65403f5 [ConventionTests] Collect all the diagnostics from PackageBuilder
Remove invalid versions
As described in SR-187,
Int(_:radix:)
(and analogous functions on other integer types) used to unintentionally accept the following cases:Where as the docs specify it returns
nil
for anything other than[-+]?[0-9a-zA-Z]
.Passes
build-script -R -t
successfully.