Skip to content

Conversation

@tlively
Copy link
Member

@tlively tlively commented Apr 5, 2024

Treat them the same as returns and test that they can be folded out of try-catch
blocks because they do not have throws effects.

@tlively
Copy link
Member Author

tlively commented Apr 5, 2024

@tlively tlively force-pushed the return-call-code-folding branch 2 times, most recently from 702e897 to e608ce4 Compare April 5, 2024 18:52
@tlively tlively changed the title [DRAFT] Handle return calls in CodeFolding Handle return calls in CodeFolding Apr 5, 2024
@tlively tlively requested a review from kripken April 5, 2024 18:53
@tlively
Copy link
Member Author

tlively commented Apr 5, 2024

cc @vouillon

@tlively tlively force-pushed the return-call-code-folding branch 2 times, most recently from 873166b to a72c996 Compare April 5, 2024 19:11
@tlively tlively force-pushed the return-call-effects branch from b4affd2 to 5ff3513 Compare April 5, 2024 19:24
@tlively tlively force-pushed the return-call-code-folding branch from a72c996 to e422e13 Compare April 5, 2024 19:24
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

lgtm % test questions

)
(unreachable)
)
)
Copy link
Member

Choose a reason for hiding this comment

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

did we not have a test for this before?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, not that I was able to find for either of these tests. Except for the throws effect analysis, the implementation does not seem to special case try-catch at all, so it's reasonable that we wouldn't need tests for the success cases. But when I was trying to debug and figure out how to get this change working, it was useful to prove to myself locally that similar cases did already work correctly.

)
)
(i32.const 0)
)
Copy link
Member

Choose a reason for hiding this comment

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

same question.

@tlively tlively force-pushed the return-call-code-folding branch 3 times, most recently from e424ec1 to 6b593a0 Compare April 5, 2024 21:56
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Sounds good, makes sense.

@tlively tlively force-pushed the return-call-code-folding branch from 6b593a0 to 837aa77 Compare April 8, 2024 16:57
Base automatically changed from return-call-effects to return-call-ctor-eval April 8, 2024 17:02
Base automatically changed from return-call-ctor-eval to interpret-return-call April 8, 2024 17:03
Base automatically changed from interpret-return-call to return-call-inlining April 8, 2024 17:04
Base automatically changed from return-call-inlining to main April 8, 2024 17:50
Treat them the same as returns and test that they can be folded out of try-catch
blocks because they do not have throws effects.
@tlively tlively force-pushed the return-call-code-folding branch from 837aa77 to dc796d4 Compare April 8, 2024 18:28
@tlively tlively merged commit 102c363 into main Apr 8, 2024
@tlively tlively deleted the return-call-code-folding branch April 8, 2024 18:52
@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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants