feat: shift operators #3019
feat: shift operators #3019
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## master #3019 +/- ##
==========================================
- Coverage 88.96% 88.73% -0.23%
==========================================
Files 84 84
Lines 10600 10636 +36
Branches 2211 2221 +10
==========================================
+ Hits 9430 9438 +8
- Misses 766 793 +27
- Partials 404 405 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
note: as this PR currently stands it uses the existing rules for this means that in the expression |
|
note to self: the fuzzer tests for this are getting sigkilled for some reason on my local machine |
|
i fixed the type checking by adding a carveout for the new shift operators during type checking. |
|
note that this changes the compile-time folding for shift to be slightly stricter than the existing |
|
@fubuloubu quick check on the docs updates and then I will merge |
| ``x >> y`` Right shift | ||
| ============= ====================== | ||
|
|
||
| Shifting is only available for 256-bit wide types. That is, ``x`` must be ``int256``, and ``y`` can be any unsigned integer. The right shift for ``int256`` compiles to a signed right shift (EVM ``SAR`` instruction). |
There was a problem hiding this comment.
I might be confused since I haven't checked yet the implementation details, but x can be uint256 OR int256 (here you say x must be int256 and below you say x must be uint256). What you probably mean (that's how I understand it) is that for logical shifting x must be uint256 whilst for arithmetic shift operations x must be int256.
Furthermore, it might be useful to add to the docs that the right shift x >> y for negative x is rounded down (towards negative infinity). The reason why I mention this is that in Solidity versions <0.5.0 the same expression (which is equivalent to the mathematical expression x / 2**y) was rounded towards zero. So we're sure people don't take wrong assumptions here.
Eventually, I also think it's important to highlight that the runtime overflow always returns 0 and not anything else. This contradicts the intuition we have from unchecked arithmetic where everything wraps and starts all over again from 0 but can have any uint256 result. What I mean by that is:
@external
@pure
def foo(x: uint8, y: uint8) -> uint8:
return unsafe_add(x, y)ExampleContract.foo(255, 255) will result in 254 and not 0.
There was a problem hiding this comment.
I might be confused since I haven't checked yet the implementation details, but x can be uint256 OR int256 (here you say x must be int256 and below you say x must be uint256). What you probably mean (that's how I understand it) is that for logical shifting x must be uint256 whilst for arithmetic shift operations x must be int256.
Not sure if I wrote it as clearly as possible, but the rationale was that the docs were already split by signed and unsigned integers, and so I split the explanations by section. The first section was already discussing signed integers, and the second section was already discussing unsigned integers.
Furthermore, it might be useful to add to the docs that the right shift x >> y for negative x is rounded down (towards negative infinity). The reason why I mention this is that in Solidity versions <0.5.0 the same expression (which is equivalent to the mathematical expression x / 2**y) was rounded towards zero. So we're sure people don't take wrong assumptions here.
Eventually, I also think it's important to highlight that the runtime overflow always returns 0 and not anything else. This contradicts the intuition we have from unchecked arithmetic where everything wraps and starts all over again from 0 but can have any uint256 result.
I don't really think this is necessary - I made sure to explain that the shift operator compiles directly to the SHR/SAR instruction, whose behavior is well known (and is also clarified at length in the text of EIP-145).
There was a problem hiding this comment.
Not sure if I wrote it as clearly as possible, but the rationale was that the docs were already split by signed and unsigned integers, and so I split the explanations by section. The first section was already discussing signed integers, and the second section was already discussing unsigned integers.
Right, that makes actually sense :)
I don't really think this is necessary - I made sure to explain that the shift operator compiles directly to the SHR/SAR instruction, whose behavior is well known (and is also clarified at length in the text of EIP-145).
Ok fair enough, but I doubt many folks even know about this EIP lol.
What I did
enable
x << yandx >> yHow I did it
added codegen rules
How to verify it
TBD
Commit message
Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)
Description for the changelog
Cute Animal Picture