Skip to content
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

[stdlib] Fix Int(_:radix:) accepting unintentional cases (SR-187) #613

Merged
merged 6 commits into from
Dec 21, 2015

Conversation

PatrickPijnappel
Copy link
Contributor

As described in SR-187, Int(_:radix:) (and analogous functions on other integer types) used to unintentionally accept the following cases:

  • Any number of minuses followed by a 0
  • A minus followed by a plus followed by any number

Where as the docs specify it returns nil for anything other than [-+]?[0-9a-zA-Z].

Passes build-script -R -t successfully.

@gribozavr
Copy link
Contributor

@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
Copy link
Contributor

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 :)

Copy link
Contributor Author

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 ;).

@PatrickPijnappel
Copy link
Contributor Author

Added terminating periods and reformatted to fit 80 columns.

gribozavr added a commit that referenced this pull request Dec 21, 2015
[stdlib] Fix Int(_:radix:) accepting unintentional cases (SR-187)
@gribozavr gribozavr merged commit 1f95076 into swiftlang:master Dec 21, 2015
@PatrickPijnappel PatrickPijnappel deleted the int-parse-fix branch December 21, 2015 02:53
gottesmm added a commit to gottesmm/swift that referenced this pull request Sep 28, 2016
…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
freak4pc pushed a commit to freak4pc/swift that referenced this pull request Sep 28, 2022
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.

2 participants