-
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
Draft
mablr
wants to merge
2
commits into
paradigmxyz:main
Choose a base branch
from
mablr:feat/sema_integer_explicit_conversions
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| //@compile-flags: -Ztypeck | ||
|
|
||
| contract IntegerConversions { | ||
| // Int <-> Int (any size - only width changes) | ||
| function validIntToInt(int8 i8, int256 i256) public pure { | ||
| int256 a = int256(i8); | ||
| int8 b = int8(i256); | ||
| int128 c = int128(i8); | ||
| int64 d = int64(i8); | ||
| int16 e = int16(i256); | ||
| } | ||
|
|
||
| // UInt <-> UInt (any size - only width changes) | ||
| function validUintToUint(uint8 u8, uint256 u256) public pure { | ||
| uint256 a = uint256(u8); | ||
| uint8 b = uint8(u256); | ||
| uint128 c = uint128(u8); | ||
| uint64 d = uint64(u8); | ||
| uint16 e = uint16(u256); | ||
| } | ||
|
|
||
| // Int <-> UInt (same size only) | ||
| function validIntUintSameSize(int8 i8, uint8 u8, int128 i128, uint128 u128) public pure { | ||
| uint8 a = uint8(i8); | ||
| int8 b = int8(u8); | ||
| uint128 c = uint128(i128); | ||
| int128 d = int128(u128); | ||
| } | ||
|
|
||
| // Int <-> UInt (different sizes - multi-aspect conversion) | ||
| // Cannot change both sign and width in one conversion | ||
| function invalidIntUintDifferentSize(int8 i8, uint256 u256, int16 i16, uint32 u32) public pure { | ||
| uint256 a = uint256(i8); //~ ERROR: cannot convert | ||
| int8 b = int8(u256); //~ ERROR: cannot convert | ||
| uint32 c = uint32(i16); //~ ERROR: cannot convert | ||
| int16 d = int16(u32); //~ ERROR: cannot convert | ||
| uint128 e = uint128(i8); //~ ERROR: cannot convert | ||
| } | ||
|
|
||
| // IntLiteral (positive) -> Int/UInt (any size) | ||
| function validPositiveLiteralToInt() public pure { | ||
| uint8 a = uint8(42); | ||
| uint16 b = uint16(256); | ||
| uint256 c = uint256(12345); | ||
| int8 d = int8(42); | ||
| int16 e = int16(100); | ||
| int256 f = int256(12345); | ||
| } | ||
|
|
||
| // Positive literals with various sizes | ||
| function validPositiveLiteralVariousSizes() public pure { | ||
| uint128 a = uint128(0); | ||
| uint32 b = uint32(0xFFFFFFFF); | ||
| int64 c = int64(9223372036854775807); // max int64 | ||
| } | ||
|
|
||
| // IntLiteral (negative) -> Int | ||
| // Note: unary minus on literals requires special handling in parser | ||
| function validNegativeLiteralToInt() public pure { | ||
| int8 a = int8(42); | ||
| int16 b = int16(256); | ||
| int256 c = int256(12345); | ||
| int128 d = int128(1); | ||
| } | ||
|
|
||
| // IntLiteral -> IntLiteral (allowed) | ||
| function validLiteralToLiteral() public pure { | ||
| int256 a = int256(uint256(42)); | ||
| uint256 b = uint256(uint128(100)); | ||
| int64 c = int64(int32(42)); | ||
| } | ||
|
|
||
| // UInt -> FixedBytes (same size) | ||
| function validUintToBytes(uint8 u8, uint32 u32, uint256 u256) public pure { | ||
| bytes1 b1 = bytes1(u8); | ||
| bytes4 b4 = bytes4(u32); | ||
| bytes32 b32 = bytes32(u256); | ||
| } | ||
|
|
||
| // UInt -> FixedBytes (different size) | ||
| function invalidUintToBytesDifferentSize(uint8 u8, uint32 u32) public pure { | ||
| bytes2 b2 = bytes2(u8); //~ ERROR: cannot convert | ||
| bytes8 b8 = bytes8(u32); //~ ERROR: cannot convert | ||
| } | ||
|
|
||
| // Nested type conversions | ||
| function validComplexConversions() public pure { | ||
| uint256 a = uint256(int256(42)); | ||
| int128 b = int128(int64(100)); | ||
| uint8 c = uint8(uint16(255)); | ||
| } | ||
|
|
||
| function validNestedConversions(int8 i8) public pure { | ||
| uint16 a = uint16(int16(i8)); | ||
| uint32 b = uint32(int32(i8)); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| error: cannot convert `int8` to `uint256` | ||
| ╭▸ ROOT/tests/ui/typeck/integer_explicit_conversions.sol:LL:CC | ||
| │ | ||
| LL │ uint256 a = uint256(i8); | ||
| ╰╴ ━━━━━━━━━━━ invalid explicit type conversion | ||
|
|
||
| error: cannot convert `uint256` to `int8` | ||
| ╭▸ ROOT/tests/ui/typeck/integer_explicit_conversions.sol:LL:CC | ||
| │ | ||
| LL │ int8 b = int8(u256); | ||
| ╰╴ ━━━━━━━━━━ invalid explicit type conversion | ||
|
|
||
| error: cannot convert `int16` to `uint32` | ||
| ╭▸ ROOT/tests/ui/typeck/integer_explicit_conversions.sol:LL:CC | ||
| │ | ||
| LL │ uint32 c = uint32(i16); | ||
| ╰╴ ━━━━━━━━━━━ invalid explicit type conversion | ||
|
|
||
| error: cannot convert `uint32` to `int16` | ||
| ╭▸ ROOT/tests/ui/typeck/integer_explicit_conversions.sol:LL:CC | ||
| │ | ||
| LL │ int16 d = int16(u32); | ||
| ╰╴ ━━━━━━━━━━ invalid explicit type conversion | ||
|
|
||
| error: cannot convert `int8` to `uint128` | ||
| ╭▸ ROOT/tests/ui/typeck/integer_explicit_conversions.sol:LL:CC | ||
| │ | ||
| LL │ uint128 e = uint128(i8); | ||
| ╰╴ ━━━━━━━━━━━ invalid explicit type conversion | ||
|
|
||
| error: cannot convert `uint8` to `bytes2` | ||
| ╭▸ ROOT/tests/ui/typeck/integer_explicit_conversions.sol:LL:CC | ||
| │ | ||
| LL │ bytes2 b2 = bytes2(u8); | ||
| ╰╴ ━━━━━━━━━━ invalid explicit type conversion | ||
|
|
||
| error: cannot convert `uint32` to `bytes8` | ||
| ╭▸ ROOT/tests/ui/typeck/integer_explicit_conversions.sol:LL:CC | ||
| │ | ||
| LL │ bytes8 b8 = bytes8(u32); | ||
| ╰╴ ━━━━━━━━━━━ invalid explicit type conversion | ||
|
|
||
| error: aborting due to 7 previous errors | ||
|
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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/:
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
Tyobjects is not enough, as we need the value fromLitKind::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
Uh oh!
There was an error while loading. Please reload this page.
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: OKIntLiteral(negative)->Int: I guess OK (but can't test it without #566)IntLiteral(positive)->Int: too restrictivei.e
int8 d = int8(0xF0);is rejected becausesize_from.bits() == 8andsize_to.bits() == 8which doesn't reflect the value bounds (−2^(n−1) to 2^(n−1) − 1)Uh oh!
There was an error while loading. Please reload this page.
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.
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).Uh oh!
There was an error while loading. Please reload this page.
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.
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::IntLiteralhere