-
Notifications
You must be signed in to change notification settings - Fork 77
feat(sema): integer explicit conversions #633
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
base: main
Are you sure you want to change the base?
feat(sema): integer explicit conversions #633
Conversation
- Added tests - Follows Solidity 0.8.0+ rules regarding type conversions.
| function validPositiveLiteralVariousSizes() public pure { | ||
| uint128 a = uint128(0); | ||
| uint32 b = uint32(0xFFFFFFFF); | ||
| int64 c = int64(9223372036854775807); // max int64 |
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.
can you also check overflow fails? same line but with 9223372036854775808
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.
That's right, mb. The check on int literal is not sufficient btw. I'll fix it to comply w/:
- Explicit conversions between literals and an integer type T are only allowed if the literal lies between type(T).min and type(T).max. [...]
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 think it's actually blocked for the moment (maybe solved by #566)
In fact we need to check the value and the sign of the literal to make this assertion : type(T).min <= literal_value < type(T).max.
So, mapping from/to Ty objects is not enough, as we need the value from LitKind::Number.
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.
would it be possible with TypeSize storing bits? that way min..=max for intX is X-1 bits and uintX is X bits
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.
That's what i tried initially, but it seems to be to restrictive given the granularity of TypeSize (byte).
IntLiteral(positive) -> Uint: OK
(IntLiteral(false, size_from), Elementary(UInt(size_to))) => { if size_from.bits() <= size_to.bits() { Ok(()) } else { Result::Err(()) }},IntLiteral(negative) -> Int: I guess OK (but can't test it without #566)
(IntLiteral(true, size_from), Elementary(Int(size_to))) => { if size_from.bits() <= size_to.bits() { Ok(()) } else { Result::Err(()) } },IntLiteral(positive) -> Int: too restrictive
(IntLiteral(false, size_from), Elementary(Int(size_to))) => { if size_from.bits() < size_to.bits() { Ok(()) } else { Result::Err(()) } },i.e int8 d = int8(0xF0); is rejected because size_from.bits() == 8 and size_to.bits() == 8 which doesn't reflect the value bounds (−2^(n−1) to 2^(n−1) − 1)
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.
right, what if typesize had bit granularity for literals? so that intX::MIN is typesize(X-1 : u16) instead of typesize(X/8 : u8)
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.
what if typesize had bit granularity for literals?
That would be great!
Correct me if I’m wrong, i think to produce Ty IntLiteral objects, it will be necessary anyway to hold the sign in the ast/hir number (the value as u256 and boolean for the sign).
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.
what if typesize had bit granularity for literals?
That would be great!
Correct me if I’m wrong, i think to produce
TyIntLiteralobjects, it will be necessary anyway to hold the sign in the ast/hir number (the value as u256 and boolean for the sign).
Yep it seems so, looking at the expression enum here
Lit only stores the numerical value, while sign is held by Unary
All these are translated in the typechecker to Ty::IntLiteral here
|
I'm going to mark this blocked by #650 (see above discussion). |
Closes #611
Follows Solidity 0.8.0+ rules regarding type conversions (see https://docs.soliditylang.org/en/latest/080-breaking-changes.html#new-restrictions)