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

Properly handle i32.trunc_f64_s of -2147483648.x #1225

Merged
merged 1 commit into from
Jul 23, 2020
Merged

Properly handle i32.trunc_f64_s of -2147483648.x #1225

merged 1 commit into from
Jul 23, 2020

Conversation

kripken
Copy link
Member

@kripken kripken commented Jul 22, 2020

Fixes #1224

Add testing for other values that seem too big to be
converted, but are ok after rounding. This is the only
one that was a spec interpreter bug. I wrote those
tests in a simple way with .9 values, replacing
the .9999999 from earlier - as @binji said, we should
either use hex-float notation, or avoid testing the parser
at the same time. As the issue here is rounding, I think
it's clearest to add .9 which suggests "this should be
rounded away".

(This is my first time writing OCaml so I'm sure I've
messed it up...)

Copy link
Member

@rossberg rossberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@kripken kripken merged commit 6d43a25 into master Jul 23, 2020
@kripken kripken deleted the conv branch July 23, 2020 22:15
gumb0 pushed a commit to wasmx/wasm-spec that referenced this pull request Sep 21, 2020
Fixes WebAssembly#1224

Add testing for other values that seem too big to be
converted, but are ok after rounding. This is the only
one that was a spec interpreter bug. I wrote those
tests in a simple way with .9 values, replacing
the .9999999 from earlier - as @binji said, we should
either use hex-float notation, or avoid testing the parser
at the same time. As the issue here is rounding, I think
it's clearest to add .9 which suggests "this should be
rounded away".
kateinoigakukun added a commit to kateinoigakukun/wasminspect that referenced this pull request Nov 3, 2021
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.

Truncating slightly less than INT32_MIN?
2 participants