Skip to content

Conversation

tlively
Copy link
Member

@tlively tlively commented Apr 27, 2024

Some of the example and gtest tests parse wat. Update them to use the new wat
parser, fixing problems with their text input. In one case, comment out an
outlining test entirely for now because the new parser produces different IR
that changes the test output, but it is not obvious how to understand and fix
the test.

Some of the example and gtest tests parse wat. Update them to use the new wat parser, fixing problems with their text input. In one case, comment out an outlining test entirely for now because the new parser produces different IR that changes the test output, but it is not obvious how to understand and fix the test.
@tlively tlively requested a review from kripken April 27, 2024 00:19
Copy link
Member Author

tlively commented Apr 27, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @tlively and the rest of your teammates on Graphite Graphite

in visitExpression for drop
adding unique symbol for End
adding unique symbol for Block Start
in visitExpression for pop i32
Copy link
Member

Choose a reason for hiding this comment

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

Why is there such a big diff here?

Copy link
Member Author

Choose a reason for hiding this comment

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

The extra two blocks mean that the top-level walker loop has two more control flow structures to pop off the control structure queue, generating more logs. The new parser also inserts a couple pops that are missing in the input that the old parser did not generate.

Copy link
Member

Choose a reason for hiding this comment

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

Which two new blocks? Sorry, I'm missing something here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, those would be the blocks the new parser generates for the catch clauses in the input. The input does not include the implicit pop instruction at the beginning of the catch clauses, so the old parser just used (drop ...) as their bodies. Since the new parser inserts the pop instruction, the catch clauses have multiple top-level instructions, so they are wrapped in a block.

(And if something seems fishy about this, it may help to know that the input program would not pass validation)

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, now I see!

@tlively tlively merged commit 85a8600 into main Apr 29, 2024
@tlively tlively deleted the parser-tests branch April 29, 2024 20:26
@gkdn gkdn mentioned this pull request Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants