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

Explicit treatment of printing mode in Yul AsmPrinter #15259

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

clonker
Copy link
Member

@clonker clonker commented Jul 12, 2024

When the YulNameRepository is merged, the AsmPrinter will need to have an instance of it anyways (which contains the dialect), so I changed the AsmPrinter s.t. the printing mode is explicitly specified instead of implicitly inferred from whether a dialect pointer is null or not.

libyul/AsmPrinter.h Outdated Show resolved Hide resolved
aarlt
aarlt previously approved these changes Jul 24, 2024
Copy link
Member

@aarlt aarlt left a comment

Choose a reason for hiding this comment

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

Looks good.

cameel
cameel previously approved these changes Jul 24, 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.

Looks good. I have some minor suggestions but nothing critical. The change seems correct and useful overall.

libyul/AsmPrinter.h Outdated Show resolved Hide resolved
libyul/AsmPrinter.cpp Outdated Show resolved Hide resolved
libyul/YulStack.cpp Outdated Show resolved Hide resolved
test/libyul/YulOptimizerTest.cpp Outdated Show resolved Hide resolved
@clonker clonker dismissed stale reviews from cameel and aarlt via 4826178 July 24, 2024 14:45
@clonker clonker force-pushed the asm_printer_mode branch 3 times, most recently from 7c3550a to 87a006c Compare July 24, 2024 14:54
@clonker
Copy link
Member Author

clonker commented Jul 24, 2024

Thanks @cameel! I implemented your suggestions.

cameel
cameel previously approved these changes Jul 24, 2024
libyul/AsmPrinter.h Outdated Show resolved Hide resolved
@clonker clonker merged commit 065c2c3 into develop Jul 25, 2024
72 checks passed
@clonker clonker deleted the asm_printer_mode branch July 25, 2024 13:52
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.

3 participants