Skip to content
This repository has been archived by the owner on Sep 9, 2024. It is now read-only.

Compile 0 literals to the PUSH0 opcode #280

Merged
merged 3 commits into from
Jul 8, 2023

Conversation

Philogy
Copy link
Contributor

@Philogy Philogy commented May 15, 2023

Overview

Closes #279. Ensures that simple zero literals like 0x0 and 0x0000 compile to PUSH0 rather than the default PUSH1 0x00. Explicit push instructions such as push1 0x00 or push21 0x0000 will not be simplified to the PUSH0 opcode giving the developer full control over how they want their pushes to be compiled.

I had trouble getting some tests to pass, but it seems to be related to missing example files? Cloning the original stage branch already did not work. With the exception of the -p huff_codegen --doc target, not sure how to fix that.

My first Huff and generally Rust PR so feedback on coding / PR style is greatly appreciated! Thx!

Copy link
Member

@Maddiaa0 Maddiaa0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah very nicely done @Philogy, excellent work especially if its your first rust work!!! Got a small note in the review section but overall i like the approach nicely done!

side note: ive made a branch that fixes the doc tests and erc20 tests here md/fix-doc-tests
Feel free to merge this branch into this pr and then ill get it merged

huff_utils/src/bytes_util.rs Show resolved Hide resolved
Copy link
Member

@clabby clabby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a fan of setting this as the default, would also be awesome to have EVM versions in Huff to branch on the version devs are deploying on for things like this. Cool to merge on my end if there's consensus around doing this prior to the compiler handling separate hardforks / conditional codegen. cc @jtriley-eth @Maddiaa0

@Maddiaa0
Copy link
Member

I'm a fan of setting this as the default, would also be awesome to have EVM versions in Huff to branch on the version devs are deploying on for things like this. Cool to merge on my end if there's consensus around doing this prior to the compiler handling separate hardforks / conditional codegen. cc @jtriley-eth @Maddiaa0

#281 have a pr here to trigger optional codegen

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants