Test coverage for constants and literal expressions with type smaller than uint256

[matheusaaguiar]

Suggested in #15968 (comment).

[github-actions]

This pull request is stale because it has been open for 14 days with no activity.
It will be closed in 7 days unless the stale label is removed.

[cameel]

Am I missing something or should both of these overflow? In either case the result is outside of uint8 range.

[cameel]

I guess the literal gets converted to the smallest integer type that can hold it rather than the type of the limited-precision operand? I did not expect that.

Looks like we do have that (somewhat) documented under Rational and Integer Literals. I think we should at some point extend these docs to have more than just a single note for such conversions and explain the rules properly.

[cameel]

This is bizarre. Also the fact that you cannot fix it with an explicit type conversion, but you can use a unintuitive workaround like base + 256 - 1 to force a bigger type and make it pass (I'd add that to the test BTW).

We really need #16420.

[cameel]

Please use capitals for constant names (e.g. BASE). This makes tests easier to read because you see at a glance what is what.

[cameel]

Do we need the layout_specification_ prefix in these tests? I see that in most cases existing tests for layout specifier do not have it. Also "specification" sounds weird in this context.

[cameel]

We need coverage also for underflow in multiplication. Consider this:

uint8 constant UNSIGNED2 = 2;
int8 constant SIGNED2 = 2;

contract S layout at SIGNED2 * -1 * -1 {  // Error: Base slot expression of type 'int8' is not convertible to uint256.
    uint[SIGNED2 * -1 * -1] a;            // OK
    uint[2 * -1 * -1] b;                  // OK
}

contract U layout at UNSIGNED2 * -1 * -1 { // Built-in binary operator * cannot be applied to types uint8 and int_const -1.
    uint[UNSIGNED2 * -1 * -1] a;           // Error: Operator * not compatible with types uint8 and int_const -1
    uint[2 * -1 * -1] b;                   // OK
}

And this shows that we were indeed missing some coverage. We have a discrepancy between layouts and arrays here. Fortunately this is not a bug in how underflow is handled but rather the fact that we require the layout expression to be unsigned, even if the value is signed.

I think the two should work the same way. IMO it would be better for both to only accept unsigned, but changing that for arrays would be breaking. We have to change it the other way then - allow signed types in the specifier.

[cameel]

I think we also need coverage for intermediate overflows and underflows:

• X + 1 - 1
• Y - 4 + 4
• X * 2 / 2
• X**2 / 255
• -~Y

where X = 255 and Y = 2 are both uint8 constants.

In unlimited precision all of these expressions would be valid.

Would not hurt to have these intermediate overflows covered for uint256 as well unless this is already tested somewhere.

[cameel]

We should also have a test verifying that division on constants is performed in limited precision and therefore truncates:

• ONE / 2 where ONE = 1 is an integer constant

This would be an error in unlimited precision.

[cameel]

We should cover shifts as well.

• SIXTEEN >> 5
• SIXTEEN << 5

where SIXTEEN = 16 is a uint8 constant.

None of these should overflow, neither in limited nor unlimited precision.

I checked this and the second one actually seems to be a buggy, because it produces an error: Error: Arithmetic error when computing constant value. This is inconsistent with our runtime calculations - if you assign the same exact expression to a variable it will not revert at runtime.