Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix[codegen]: range bound check for signed integers #3814

Merged
merged 4 commits into from
Feb 28, 2024

Conversation

DanielSchiavini
Copy link
Contributor

What I did

  • Fixed range overflow when using signed integers

How I did it

  • By checking if the target type is signed

How to verify it

  • Tests included

Commit message

Fix range overflow when using signed integers

Description for the changelog

Fix range overflow when using signed integers

Cute Animal Picture

image

@DanielSchiavini DanielSchiavini changed the title Fix range overflow when using signed integers fix: range overflow when using signed integers Feb 26, 2024
@codecov-commenter
Copy link

codecov-commenter commented Feb 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.22%. Comparing base (730679b) to head (b6f87d4).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3814      +/-   ##
==========================================
+ Coverage   85.19%   85.22%   +0.03%     
==========================================
  Files          92       92              
  Lines       13914    13916       +2     
  Branches     3118     3118              
==========================================
+ Hits        11854    11860       +6     
+ Misses       1565     1561       -4     
  Partials      495      495              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

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

haha, we used to have dedicated clample instructions in the IR before i removed them (they did something slightly different than here though).

i think a bit longer-term the right refactor here is to have dedicated helper functions like def LE(is_signed), def GT(is_signed): etc. but it's probably out of scope for this PR.

@DanielSchiavini could you merge in the CI fixes from master? and then i can merge

@charles-cooper charles-cooper enabled auto-merge (squash) February 28, 2024 13:11
@charles-cooper charles-cooper changed the title fix: range overflow when using signed integers fix[codegen]: range bound check for signed integers Feb 28, 2024
@charles-cooper charles-cooper merged commit b4429cf into vyperlang:master Feb 28, 2024
84 checks passed
@DanielSchiavini DanielSchiavini deleted the 3768/range_bound branch February 28, 2024 15:07
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants