eof: Update yulSyntaxTests tests for EOF
#15660
Conversation
rodiazet
commented
Dec 19, 2024
rodiazet
commented
- Disable test which don't make sense for EOF bytecode format.
- Create EOF yul syntax tests counterparts.
|
Thank you for your contribution to the Solidity compiler! A team member will follow up shortly. If you haven't read our contributing guidelines and our review checklist before, please do it now, this makes the reviewing process and accepting your contribution smoother. If you have any questions or need our help, feel free to post them in the PR or talk to us directly on the #solidity-dev channel on Matrix. |
rodiazet
force-pushed
the
eof-update-yul-syntax-tests
branch
from
914dbb9 to
2ca82d3
Compare
There was a problem hiding this comment.
We should have a version of this for EOF, using one of the EOF builtins, e.g. auxdataloadn().
There was a problem hiding this comment.
Same with passing_builtin_with_literal_argument_into_literal_argument.yul.
There was a problem hiding this comment.
Same situation as #15661 (comment): replace codesize() with calldataload(0) and it will work on EOF too.
Same for hex_switch_case_long.yul and string_literal_switch_case.yul.
There was a problem hiding this comment.
And the three opcode_for_function*.yul tests will work fine with EOF if you switch from gas to mload for example.
There was a problem hiding this comment.
We should have a test for accessing .metadata (and more generally data starting with a dot like in metadata_access_2.yul and metadata_access_subobject.yul) with auxdataloadn(). Is it actually disallowed as it should?
There was a problem hiding this comment.
We should have EOF variants of these as well (using auxdataloadn()):
data.yul(accessing data objects longer than 32-bytes or using hex)subobject_access.yul(accessing non-data objects - should be forbidden)- accessing data nested more than one level.
There was a problem hiding this comment.
Disabling tests for legacy-only opcodes here is fine, but just a reminder that we'll still need to have them covered eventually, the way I described in #15559 (comment).
There was a problem hiding this comment.
Actually, while that test is not critical, we still haven't fixed the underlying issue. Those identifiers are still reserved in Yul and we absolutely have to fix that before the next release. I created an issue so that it does not get forgotten: #15672.
