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

make String.f32() and String.f64() partial #3043

Merged
merged 1 commit into from
Mar 1, 2019
Merged

Conversation

mfelsche
Copy link
Contributor

@mfelsche mfelsche commented Mar 1, 2019

and fail if the string at the given offset contains an invalid F32/F64
which includes the empty string and trailing characters.

previously they were returning 0 in their respective return-type. So it was impossible to find out if parsing failed.

fixes #3026

and fail if the string at the given offset contains an invalid F32/F64
which includes the empty string and trailing characters.
@mfelsche mfelsche added the changelog - changed Automatically add "Changed" CHANGELOG entry on merge label Mar 1, 2019
@mfelsche
Copy link
Contributor Author

mfelsche commented Mar 1, 2019

Hey, everything succeeded but the docker build check failed because it cant find some makefile. That is unrelated to this PR and needs some separate investigation.

@SeanTAllen
Copy link
Member

@mfelsche its because your PR isnt against the latest master.

@SeanTAllen SeanTAllen added the do not merge This PR should not be merged at this time label Mar 1, 2019
@SeanTAllen
Copy link
Member

Please do not merge this prior to the 0.27.0 release. There's no release notes with "before and after" code for the breaking change and I don't want to hold up the release that I am planning on doing within 4-5 hours.

@mfelsche
Copy link
Contributor Author

mfelsche commented Mar 1, 2019

@SeanTAllen here are my release notes:

This change changes the methods builtin.String.f32(offset: ISize) and builtin.String.f64(offset: ISize) to be partial and fail on input that cannot be parsed as a floating point number. This is the current behavior already for parsing integers from Strings.

Previously the methods for parsing floats would just return 0.0 on invalid input, overflow or when the given offset was beyond the bounds of the String it was invoked on. As 0.0 is a valid float, there was no real way to determine if the parsing failed or if 0.0 is a legit parsed value. These examples try to show the behavior of Pony prior to 0.27.0:

// true with Pony 0.26.0 and older
"".f32() == F32(0)
"ABC".f64() == F64(0)
"0.123".f64(10) == F64(0) 

The following example shows the new behavior with Pony 0.27.0 and newer:

// these expressions will error with Pony 0.27.0 and newer
try "".f32()? end
try "ABC".f64()? end
try "0.123".f64(10)? end 

@SeanTAllen SeanTAllen removed the do not merge This PR should not be merged at this time label Mar 1, 2019
@SeanTAllen SeanTAllen merged commit cfd423f into master Mar 1, 2019
@SeanTAllen SeanTAllen deleted the string-float branch March 1, 2019 18:45
ponylang-main added a commit that referenced this pull request Mar 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - changed Automatically add "Changed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Int parsing methods on strings error on non-numeric input, Float parsing methods return 0
3 participants