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.

Sign up for GitHub

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

Jump to bottom

CLI output selection in assembler mode #12074

Merged
merged 2 commits into from
Nov 8, 2021
Merged

CLI output selection in assembler mode #12074

merged 2 commits into from
Nov 8, 2021

Conversation

cameel
Copy link
Member

@cameel cameel commented Oct 1, 2021 •
edited
Loading

Resolves #3870.
Depends on #12071. In draft mode until that PR is merged. Merged.

This does not add any new outputs, just makes the ones that are already available respond to output selection flags so e.g. --ir is not available. We can

Also, to make this non-breaking, all outputs are selected if no flags are specified, which is different from how the compiler mode behaves. If this goes through, I can create a separate PR to breaking branch to change that.

@cameel cameel self-assigned this Oct 1, 2021
@cameel cameel marked this pull request as draft October 1, 2021 12:54
This was referenced Oct 1, 2021
Closed
Merged
@cameel cameel force-pushed the cli-validate-output-selection branch 2 times, most recently from a3fa5d7 to b9b2c69 Compare October 6, 2021 18:09
Base automatically changed from cli-validate-output-selection to develop October 11, 2021 15:52
@cameel
Copy link
Member Author

cameel commented Oct 11, 2021

#12071 merged so this is now reviewable.

@cameel cameel marked this pull request as ready for review October 11, 2021 15:53
@cameel cameel force-pushed the output-selection-in-assembler-mode branch 2 times, most recently from 88630cf to dd0f837 Compare October 11, 2021 18:36
@cameel cameel changed the base branch from develop to fix-circleci-macos-cache-key October 11, 2021 18:36
@cameel
Copy link
Member Author

cameel commented Oct 11, 2021 •
edited
Loading

Note: this is now based on the CI fix (#12106).

Base automatically changed from fix-circleci-macos-cache-key to develop October 12, 2021 14:22
@cameel cameel force-pushed the output-selection-in-assembler-mode branch 2 times, most recently from 9964dd6 to c42266d Compare October 14, 2021 15:47
@cameel cameel mentioned this pull request Oct 14, 2021
Merged
@cameel cameel force-pushed the output-selection-in-assembler-mode branch from c42266d to 7f3bba0 Compare October 15, 2021 13:51
This was referenced Oct 15, 2021
Closed
Closed
@cameel cameel force-pushed the output-selection-in-assembler-mode branch from 7f3bba0 to e33c095 Compare November 2, 2021 15:33
leonardoalt
leonardoalt previously approved these changes Nov 3, 2021
View reviewed changes
solc/CommandLineInterface.cpp Outdated
sout() << endl << "==========================" << endl;
sout() << endl << "Translated source:" << endl;
sout() << stack.print() << endl;
// TODO: This isn't ewasm but it's only present when we're doing Yul->EWASM translation.
Copy link
Member

Choose a reason for hiding this comment

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

I guess this is not going to be done in this PR? Maybe create an issue so it doesn't get forgotten?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, I just realized that ewasm is still experimental. So maybe it's not breaking after all?

Copy link
Member Author

Choose a reason for hiding this comment

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

Got a confirmation on the chat. It's not breaking.

In that case I'm adding an extra commit that adds an output called --ewasm-ir (I could not come up with a better name).

solc/CommandLineParser.cpp Show resolved Hide resolved
@cameel cameel mentioned this pull request Nov 3, 2021
Closed
@cameel cameel force-pushed the output-selection-in-assembler-mode branch from e33c095 to ed7e986 Compare November 3, 2021 16:07
@cameel cameel requested a review from leonardoalt November 3, 2021 16:10
leonardoalt
leonardoalt previously approved these changes Nov 3, 2021
View reviewed changes
@leonardoalt
Copy link
Member

wait, do the tests need update?

@cameel cameel force-pushed the output-selection-in-assembler-mode branch from ed7e986 to 9cd4e3c Compare November 3, 2021 17:08
@cameel
Copy link
Member Author

cameel commented Nov 3, 2021

I did update them but looks like I missed one change needed after a rebase. Apparently an extra stop instruction gets added after sstore due to #11974. There are other similar changes in that PR so it seems legit.

Should be fixed now.

@cameel cameel force-pushed the output-selection-in-assembler-mode branch from 9cd4e3c to 6b605b5 Compare November 3, 2021 18:53
@cameel cameel force-pushed the output-selection-in-assembler-mode branch from 6b605b5 to 1a19d9a Compare November 4, 2021 17:23
@cameel cameel merged commit 44f7065 into develop Nov 8, 2021
@cameel cameel deleted the output-selection-in-assembler-mode branch November 8, 2021 15:59
@cameel cameel mentioned this pull request Nov 9, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leonardoalt leonardoalt leonardoalt approved these changes

Assignees

@cameel cameel

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CLI] Do not ignore the output selection in assembler mode
2 participants
@cameel @leonardoalt