Skip to content

Remove EOF: libyul#16770

Open
rodiazet wants to merge 1 commit into
developfrom
remove-eof-libyul
Open

Remove EOF: libyul#16770
rodiazet wants to merge 1 commit into
developfrom
remove-eof-libyul

Conversation

@rodiazet
Copy link
Copy Markdown
Contributor

@rodiazet rodiazet commented May 22, 2026 •
edited
Loading

Remove EOF from libyul

  • No AI tools were used
  • AI tools were used (details below)

Depends on: #16769

@rodiazet rodiazet added the has dependencies The PR depends on other PRs that must be merged first label May 22, 2026
@rodiazet rodiazet force-pushed the remove-eof-libyul branch from 555294a to bfd7665 Compare May 22, 2026 14:35
@rodiazet rodiazet mentioned this pull request May 25, 2026
Open
2 tasks
@rodiazet rodiazet added the EOF label May 25, 2026
matheusaaguiar
matheusaaguiar previously approved these changes May 26, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@matheusaaguiar matheusaaguiar left a comment

Choose a reason for hiding this comment

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

LGTM

@rodiazet rodiazet force-pushed the remove-eof-libsolidity branch from 088861a to ed5a8ab Compare May 27, 2026 12:00
@rodiazet rodiazet force-pushed the remove-eof-libyul branch from bfd7665 to 3f8bb14 Compare May 27, 2026 12:00
@rodiazet rodiazet force-pushed the remove-eof-libsolidity branch 6 times, most recently from 40c0d22 to c06a4e2 Compare May 28, 2026 15:20
@rodiazet rodiazet force-pushed the remove-eof-libyul branch 4 times, most recently from 9501ad8 to b5876bf Compare May 28, 2026 20:43
Base automatically changed from remove-eof-libsolidity to develop May 29, 2026 06:37
@nikola-matic nikola-matic dismissed matheusaaguiar’s stale review May 29, 2026 06:37

The base branch was changed.

@nikola-matic
Copy link
Copy Markdown
Contributor

nikola-matic commented May 29, 2026

@rodiazet this is the next one, right?

@rodiazet
Copy link
Copy Markdown
Contributor Author

rodiazet commented May 29, 2026

@rodiazet this is the next one, right?

Yes

@rodiazet rodiazet removed the has dependencies The PR depends on other PRs that must be merged first label May 29, 2026
@rodiazet rodiazet force-pushed the remove-eof-libyul branch from b5876bf to 0168407 Compare May 29, 2026 10:22
matheusaaguiar
matheusaaguiar previously approved these changes May 29, 2026
View reviewed changes
blishko
blishko requested changes Jun 1, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@blishko blishko left a comment

Choose a reason for hiding this comment

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

I believe the PROFILER_PROBE change has to be fixed. Otherwise, I just have a few questions to help me clarify things.

Comment thread libevmasm/Assembly.cpp
Comment thread libyul/backends/evm/OptimizedEVMCodeTransform.cpp Outdated

void OptimizedEVMCodeTransform::appendDup(size_t _depth)
{
if (_depth <= 16)
Copy link
Copy Markdown
Contributor

@blishko blishko Jun 1, 2026

Choose a reason for hiding this comment

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

Should 16 be hardcoded here like this?

Copy link
Copy Markdown
Contributor

@blishko blishko Jun 1, 2026

Choose a reason for hiding this comment

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

Also, dupInstruction also has an assert for this. Maybe we can rely on that assert? Or is it beneficial to have a separate check here?

Copy link
Copy Markdown
Contributor Author

@rodiazet rodiazet Jun 2, 2026

Choose a reason for hiding this comment

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

It was there like this before, but I agree that we can rely on the assert in the dupInstruction and swapInstruction.

Comment thread libyul/optimiser/Suite.cpp Outdated
Comment thread libyul/ObjectOptimizer.cpp
Comment thread test/Common.cpp
Comment thread test/TestCase.cpp
@rodiazet rodiazet force-pushed the remove-eof-libyul branch from 0168407 to 4b902db Compare June 2, 2026 08:41
@rodiazet rodiazet requested review from blishko and cameel June 2, 2026 08:45
@rodiazet rodiazet requested a review from nikola-matic June 2, 2026 08:46
blishko
blishko reviewed Jun 2, 2026
View reviewed changes
Comment thread libyul/backends/evm/OptimizedEVMCodeTransform.cpp

void OptimizedEVMCodeTransform::appendSwap(size_t _depth)
{
if (_depth <= 16)
Copy link
Copy Markdown
Contributor

@blishko blishko Jun 2, 2026

Choose a reason for hiding this comment

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

I have actually realized that maybe we should keep this if for now. Basically like you had it before this latest changes.
I have not thought about the static_cast before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@blishko blishko blishko requested changes

@matheusaaguiar matheusaaguiar matheusaaguiar left review comments

@cameel cameel Awaiting requested review from cameel

@nikola-matic nikola-matic Awaiting requested review from nikola-matic

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

EOF

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rodiazet @nikola-matic @blishko @matheusaaguiar