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

Added back Yul tests that were fixed by Solidity #830

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hrkrshnn
Copy link

@hrkrshnn hrkrshnn commented Oct 5, 2021

Description

Solidity fixes all the issues found: ethereum/solidity#9500

Did not test it locally :( Hopefully the CI can do it?

Checklist

  • tested locally
  • added automated tests
  • updated the docs
  • updated the changelog

@hrkrshnn
Copy link
Author

hrkrshnn commented Oct 5, 2021

Also how can I test this locally?

@hrkrshnn
Copy link
Author

hrkrshnn commented Oct 6, 2021

The tests are failing, but they are not matching with the tests from the solidity repo:

executing test: varNameCleaner/function_names.yul with z3
Could not compile second Yul source. (/build/tmp.05kOOioBc4)
{
calldatacopy(0,0,1024)
    let f_1
    function f()
    { let f_1 }
    let f_2
}

The test was updated in 0.7.1: https://github.com/ethereum/solidity/blob/develop/test/libyul/yulOptimizerTests/varNameCleaner/function_names.yul

Any chance that it's pulling from the old repo?

@d-xo
Copy link
Contributor

d-xo commented Oct 7, 2021

Any chance that it's pulling from the old repo?

yeah we're still using 0.6.7 for the tests:

rev = "b8d736ae0c506b1b3cf5d2456af67e8dc2c0ca8e"; # v0.6.7
, if you're feeling motivated you can bump that to 0.8.7 and then there will presumably be some work needed to categorise any new tests that might be failing.

Also how can I test this locally?

nix-build -A hevm-tests from the repo root.

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

Successfully merging this pull request may close these issues.

2 participants