Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eof: Disable smtCheckerTests when compiling to EOF #15659

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

rodiazet
Copy link
Contributor

All smtCheckerTests are run for current EVM version which is incompatible with EOF.

Copy link

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 rodiazet force-pushed the eof-disable-smt-checket-test branch from 0a1ad6f to 46e40ac Compare December 19, 2024 15:24
@cameel cameel changed the title eof: Disable smtCheckeTests when compiling to EOF. eof: Disable smtCheckerTests when compiling to EOF Dec 19, 2024
cameel
cameel previously approved these changes Dec 19, 2024
Copy link
Member

@cameel cameel left a comment

Choose a reason for hiding this comment

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

Generally ok, but needs a small tweak.

test/libsolidity/SMTCheckerTest.cpp Outdated Show resolved Hide resolved
Comment on lines 140 to 142
if (m_evmVersion < langutil::EVMVersion::prague() && CommonOptions::get().eofVersion().has_value())
m_shouldRun = false;
Copy link
Member

Choose a reason for hiding this comment

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

Please add a TODO to enable it when EOF is stable - to make it clear this is only temporary, not the desired state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Good to mention also that first we need to run smt checker tests via IR first.

Copy link
Member

Choose a reason for hiding this comment

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

True. I guess the code generation backend used is mostly irrelevant to SMTChecker which is we don't even run it via IR. For EOF it would be relevant where it introduces new syntax

@cameel cameel merged commit 2462dfa into ethereum:develop Dec 20, 2024
73 checks passed
@cameel cameel mentioned this pull request Dec 21, 2024
61 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants