lann commented on issue #5510:
I failed to mention: this removes the
#[derive(Eq)]
fromVal
and some related structs becausef{32,64}
aren'tEq
. I think this makes sense in this context but its technically a breaking change.
lann edited a comment on issue #5510:
I failed to mention: this removes the
#[derive(Eq)]
fromVal
and some related structs becausef{32,64}
aren'tEq
. I think this makes sense in this context but it's technically a breaking change.
alexcrichton commented on issue #5510:
Yeah that's ok and I think it makes sense as well due to the usage of floats it shouldn't be applicable for
Val
anyway.
github-actions[bot] commented on issue #5510:
Subscribe to Label Action
cc @fitzgen, @peterhuene
<details>
This issue or pull request has been labeled: "fuzzing", "wasmtime:api"Thus the following users have been cc'd because of the following labels:
- fitzgen: fuzzing
- peterhuene: wasmtime:api
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
alexcrichton commented on issue #5510:
It looks like this broke fuzzing here because nan is no longer equal to nan. @lann would you be up for helping to fix this? I think the way to fix it would be to write a custom equality function which considers two NaN values to be equal.
lann commented on issue #5510:
Would it make sense to change the test code? e.g.
match (expected, actual) { // NaNs never compare equal (Val::Float32(expected), Val::Float32(actual)) if expected.is_nan() => assert!(actual.is_nan()), (Val::Float64(expected), Val::Float64(actual)) if expected.is_nan() => assert!(actual.is_nan()), (expected, actual) => assert_eq!(expected, actual), };
alexcrichton commented on issue #5510:
More-or-less that's what needs to happen, but floats can appear recursively as well so the equality change there would need to be applied deeply through types like lists and recors too
Last updated: Nov 22 2024 at 16:03 UTC