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.

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

Changed the todo's to propagate the NaN for multiplication #42

Closed
wants to merge 1 commit into from
Closed

Changed the todo's to propagate the NaN for multiplication #42

wants to merge 1 commit into from

Conversation

MidasLamb
Copy link
Contributor

The multiplication had some todo's left for NaN, made it propagate the NaN.

@MidasLamb
Changed the todo's to propagate the NaN for multiplication
510b5c0 
The multiplication had some todo's left for NaN, made it propagate the
NaN.
@connorskees
Copy link
Owner

Thank you for your work on this, and fuzzing grass in general. This change has been superseded by work in #67 that is finally close to being merged. This change unfortunately doesn't take into account number units, and so while it does resolve the crash, it introduces a silent miscompilation. In #67 I've changed the number representation to just be f64, and so the special casing of NaN and Infinity is no longer necessary.

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
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MidasLamb @connorskees