-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Unify Yul parsing across test suites #15611
Merged
Merged
+280
−366
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
48afef8
to
b0efac5
Compare
812efab
to
609fd43
Compare
7f49fb7
to
e3a35e8
Compare
609fd43
to
4d3140e
Compare
e3a35e8
to
cbe7782
Compare
48fd3d0
to
29c2a39
Compare
b051766
to
f0656c4
Compare
29c2a39
to
f1e44cf
Compare
f0656c4
to
ace62cd
Compare
e797b3c
to
fcbfbf5
Compare
clonker
reviewed
Dec 12, 2024
clonker
approved these changes
Dec 12, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't find anything wrong with the PR, if you want to expose the dialect in the YulStack
I'll reapprove afterwards
fcbfbf5
to
9acd665
Compare
clonker
previously approved these changes
Dec 13, 2024
ace62cd
to
47545d3
Compare
Base automatically changed from
fix-code-generation-error-reporting-in-yul
to
develop
December 13, 2024 13:36
9acd665
to
3d8ff6b
Compare
3d8ff6b
to
2f4d58f
Compare
2f4d58f
to
46b0616
Compare
clonker
approved these changes
Dec 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #15610.Merged.Just a test refactor in preparation for another PR.
Currently most Yul test suites use the
yul::test::parse()
helper, which has two overloads which do things in wildly inconsistent ways. One usesYulStack
but assumes no errors and reports only the top-level AST. The other manually runsObjectParser
and does return errors and the wholeObject
but only runs analysis on the top-level AST ignoring the others.On top of that neither of them really suits my needs, because they don't allow continuing the compilation after parsing. The PR replaces them with a new, more flexible, helper that returns
YulStack
instead. Then makes all test suites use it.Also:
dialect
setting but there's only one dialect and passing it toYulStack
is a bit of a hassle so not having to do that simplifies things a lot.