Skip to content

tryparse should still throw an exception on invalid base #17333

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 9, 2016
Merged

tryparse should still throw an exception on invalid base #17333

merged 1 commit into from
Jul 9, 2016

Conversation

stevengj
Copy link
Member

@stevengj stevengj commented Jul 8, 2016

tryparse does not throw an exception on an invalid string, but it should still throw an ArgumentError if the base argument is invalid. This fixes a regression apparently introduced by #17078.

In particular, this regression has broken the Compat.jl tests (see e.g. JuliaLang/Compat.jl#245, JuliaLang/Compat.jl#240).

cc: @yuyichao, @quinnj

@stevengj stevengj added the regression Regression in behavior compared to a previous version label Jul 8, 2016
@stevengj
Copy link
Member Author

stevengj commented Jul 8, 2016

Oh grr, we need to distinguish the base passed by the user (which should throw) from the base extracted by parseint_preamble (which shouldn't throw).

@stevengj
Copy link
Member Author

stevengj commented Jul 8, 2016

Okay, should be fixed now, I hope.

@stevengj stevengj mentioned this pull request Jul 8, 2016
Merged
@stevengj
Copy link
Member Author

stevengj commented Jul 8, 2016

AppVeyor failure in pkg test seems to be an unrelated network problem: GitError(Code:ERROR, Class:OS, Failed to receive response: The server returned an invalid or unrecognized response).

@quinnj
Copy link
Member

quinnj commented Jul 8, 2016

LGTM

@tkelman tkelman merged commit 8377220 into JuliaLang:master Jul 9, 2016
@stevengj stevengj deleted the tryparse_throw branch July 9, 2016 12:07
@ranjanan ranjanan mentioned this pull request Jul 11, 2016
Merged
@stevengj stevengj mentioned this pull request Jul 11, 2016
Merged
mfasi pushed a commit to mfasi/julia that referenced this pull request Sep 5, 2016
@stevengj @mfasi
tryparse should still throw an exception on invalid base (JuliaLang#1…
ceccd71 
…7333)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
regression Regression in behavior compared to a previous version
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@stevengj @quinnj @tkelman