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

Add support for casting between numeric types #576

Merged
merged 1 commit into from
Oct 22, 2021

Conversation

cburgdorf
Copy link
Collaborator

@cburgdorf cburgdorf commented Oct 20, 2021

What was wrong?

There's currently no way to convert between different numeric types.

How was it fixed?

Added support for explicit type casting such as u16(some_u8_value). Type casting between any numeric types is allowed (which means types may be truncated) but it is a type error to change both the sign and the size at the same time. This is inspired by Solidity which handles it the same way. It does mean that we diverge from Rust here but I think that being more strict is the right thing for us to do here.

@cburgdorf cburgdorf force-pushed the christoph/feat/convert_numerics branch from 2fbefd1 to 98ad76d Compare October 20, 2021 13:33
#
# Notice that Rust does allow the transition in one
# step and apparently size is changed before the sign:
# https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=cfc82437a365ca1f534a40f5182bd1e2
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This whole thing caught me by surprise. Found it in the solidity docs and then validated it with Rust. Decided that it would be good to follow Solidity with that stricter checking.

@cburgdorf cburgdorf force-pushed the christoph/feat/convert_numerics branch from 98ad76d to 9dbdb65 Compare October 21, 2021 16:15
Copy link
Collaborator

@sbillig sbillig left a comment

Choose a reason for hiding this comment

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

Looks good. I pushed a fix for the new clippy lint directly to master (:astonished:); I'll merge this when the CI checks pass on that commit.

@sbillig sbillig merged commit e9c1ee1 into ethereum:master Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants