-
Notifications
You must be signed in to change notification settings - Fork 187
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 numeric invert operator #526
Add support for numeric invert operator #526
Conversation
86889d9
to
6add282
Compare
case("return_invert_i64.fe", &[int_token(4000000000)], int_token(-4000000001)), | ||
case("return_invert_i32.fe", &[int_token(30000)], int_token(-30001)), | ||
case("return_invert_i16.fe", &[int_token(2000)], int_token(-2001)), | ||
case("return_invert_i8.fe", &[int_token(1)], int_token(-2)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a special use for signed ints that I'm missing?
Since this is a bitwise operation, I think use should be restricted to unsigned ints, just like the binary bitwise operations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't thought much about use cases but Solidity, Python, Rust (and probably other languages) all let you do this so I think we shouldn't be overly restrictive.
Solidity
int8 y = -10;
assert(~y == 9);
Python:
>>> ~-10
9
Rust:
assert_eq!(!-10i8, 9);
let fn_name = names::adjust_numeric_size(&size); | ||
function_definition! { | ||
function [fn_name](value) -> cleaned { | ||
(cleaned := signextend([literal_expression! { (base) }], value)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, this solution looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just copied what Solidity is doing 😄 We still need to apply that to a bunch of other places (which I will follow up on after this PR)
6add282
to
4073cd4
Compare
What was wrong?
As noted in #521 we needed to add support for the
~
operator which is currently recognized on the parser level but not analyzed and generates no code.How was it fixed?
Implementation is pretty straight forward. The more interesting part is that this PR lays the ground work for a bunch of new
adjust_numeric_*
runtime functions that we need to use at a bunch more places to ensure numerics smaller than 256 bit are handled correctly. This PR only uses these methods for the unary invert operator.