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

jsgen: fix incorrect float32-literal behaviour #824

Merged
merged 1 commit into from
Aug 3, 2023

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Aug 2, 2023

Summary

Narrow the value of 32-bit float-literal nodes prior to rendering them,
which fixes float32 literals having an unexpected run-time value
when their value as-written is not fully representable with the
precision of a 32-bit float.

Details

Instead of using toStrMaxPrecision, which would add an unwanted f
suffix, rendering the float value to text is done by directly calling
addFloatRoundtrip, which toStrMaxPrecision uses internally.

Summary
=======

Narrow the value of 32-bit float-literal nodes prior to rendering them,
which fixes `float32` literals having an unexpected run-time value
when their value as-written is not fully representable with the
precision of a 32-bit float.

Details
=======

Instead of using `toStrMaxPrecision`, which would add an unwanted `f`
suffix, rendering the float value to text is done by directly calling
`addFloatRoundtrip`, which `toStrMaxPrecision` uses internally.
@zerbina zerbina added bug Something isn't working compiler/backend Related to backend system of the compiler labels Aug 2, 2023
@zerbina zerbina added this to the JS backend refactoring milestone Aug 2, 2023
Copy link
Collaborator

@saem saem left a comment

Choose a reason for hiding this comment

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

nice and things are more uniform to boot.

@saem
Copy link
Collaborator

saem commented Aug 2, 2023

/merge

@github-actions
Copy link

github-actions bot commented Aug 2, 2023

Merge requested by: @saem

Contents after the first section break of the PR description has been removed and preserved below:


Notes for Reviewers

@chore-runner chore-runner bot added this pull request to the merge queue Aug 2, 2023
Merged via the queue into nim-works:devel with commit 50c2ac0 Aug 3, 2023
@zerbina zerbina deleted the jsgen-fix-float32-rendering branch August 6, 2023 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler/backend Related to backend system of the compiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants