Skip to content
This repository has been archived by the owner on Oct 28, 2021. It is now read-only.

Update LLL code in tests to avoid using define-on-use variables #3401

Merged
merged 2 commits into from
Nov 29, 2016

Conversation

axic
Copy link
Member

@axic axic commented Nov 5, 2016

This change should generate the exact same bytecode as before.

Fixes #3387.

@axic
Copy link
Member Author

axic commented Nov 5, 2016

@winsvega can you please review this? It should work with the "strict parser" branch of Solidity.

@zigguratt can you also please double check this?

@zigguratt
Copy link

I found a couple more places where i is auto-defined. I don't have commit privileges of course, so I've attached a diff (apparently GitHub doesn't recognize .diff files).

out.diff.txt

@axic axic force-pushed the tests-lll-for-fixes branch from a148e2b to 7f7c9b1 Compare November 5, 2016 14:17
@axic
Copy link
Member Author

axic commented Nov 5, 2016

@zigguratt thank you, added your changes

@chriseth
Copy link
Contributor

chriseth commented Nov 7, 2016

@winsvega is this fine from your side?

@chfast
Copy link
Member

chfast commented Nov 9, 2016

Does this modify the output tests?

@axic
Copy link
Member Author

axic commented Nov 9, 2016

@chfast the changed code should output the exact same bytecode and should work with both the current lllc and with this PR: ethereum/solidity#1313.

I just don't have the knowledge (and CPU power) on the testing infrastructure, to run the test generators (fillers?) and the actual tests to verify.

@winsvega
Copy link
Contributor

could we merge this into #3381
eip158 branch ?

@axic
Copy link
Member Author

axic commented Nov 14, 2016

@winsvega should I push this commit to that branch?

@chriseth
Copy link
Contributor

@winsvega please add the correct transition block number, add the strict lll parser commit by @axic and then merge.

@axic
Copy link
Member Author

axic commented Nov 22, 2016

@winsvega can you please confirm this doesn't break any tests (with both the stock lllc and the solidity/#1313 PR)?

@axic axic force-pushed the tests-lll-for-fixes branch from 7f7c9b1 to 2e1f511 Compare November 28, 2016 12:06
@axic
Copy link
Member Author

axic commented Nov 28, 2016

@winsvega @chriseth updated for EIP158

Copy link
Contributor

@winsvega winsvega left a comment

Choose a reason for hiding this comment

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

The tests would be upgraded. I am afraid we will have to repeat this changes to the new general tests

@winsvega winsvega merged commit ba6285b into develop Nov 29, 2016
@axic axic deleted the tests-lll-for-fixes branch November 29, 2016 11:41
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.

5 participants