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

semfold: fix range-check folding #822

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Jul 31, 2023

Summary

Fix literals resulting from folding range-check conversions having a
type that doesn't match the node kind. This, in turn, fixes a VM code
generator issue where the code generated for an access of the result of
an integer-literal-converted-to-float-range would cause the VM to crash.

Details

The folding logic directly assigned the destination type to the literal,
which led to, for example, integer literals having a tyFloat type.
This breaks the assumption of only float-literal nodes having a float
type, and is what caused the vmgen bug (vmgen only looks at the node
kind).

Same as for normal conversion, range-check conversion are now also
folded via foldConv (but only in case the range check succeeds),
fixing the issue.

Summary
=======

Fix literals resulting from folding range-check conversions having a
type that doesn't match the node kind. This, in turn, fixes a VM code
generator issue where the code generated for an access of the result of
an integer-literal-converted-to-float-range would cause the VM to crash.

Details
=======

The folding logic directly assigned the destination type to the literal,
which led to, for example, integer literals having a `tyFloat` type.
This breaks the assumption of only float-literal nodes having a float
type, and is what caused the `vmgen` bug (`vmgen` only looks at the node
kind).

Same as for normal conversion, range-check conversion are now also
folded via `foldConv` (but only in case the range check succeeds),
fixing the issue.
@zerbina zerbina added bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler labels Jul 31, 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.

The explicit conversion and applying the type there makes more sense.

@saem
Copy link
Collaborator

saem commented Jul 31, 2023

/merge

@github-actions
Copy link

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 Jul 31, 2023
Merged via the queue into nim-works:devel with commit b794757 Jul 31, 2023
@zerbina zerbina deleted the fix-range-check-folding 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/sem Related to semantic-analysis system of the compiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants