Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_control_flow): introduce explicit exception control flow #2821

Merged
merged 1 commit into from
Jul 7, 2022

Conversation

leops
Copy link
Contributor

@leops leops commented Jul 5, 2022

Summary

This PR replaces the (temporary) initial implementation of try-catch-finally statements in the control flow analysis (based on the concept of multiple entry points) with a more complete modelling solution for the exception control flow. The BasicBlock struct that make up the control flow graph now also contain a list of "exception handlers" and "cleanup handlers": these handlers point to a set of blocks to be executed in order when an exception is thrown (either with an explicit throw instruction or as a possible side-effect of a statement) or when the function returns respectively.

For JavaScript, the exceptions handlers of a block are all the finally clauses found going upward in the syntax tree until a catch clause is found (the block corresponding to this clause is the last handler in the list), while the cleanup handlers are made of all the finally clauses going up to the root level of the function (and ignoring all catch clauses)

The last change I've made is introducing a new finally_fallthrough flag on Jump instructions. This flag is used specifically to mark the implicit jump instruction inserted by the control flow generator at the end of a finally block towards the next statement: when the finally clause gets executed as an exception or cleanup handler instead of going through the normal control flow the target block of this jump gets modified to the next handler in the chain if it exists, or as a return instruction otherwise.

Test Plan

I've added additional test cases for JsTryStatement and JsTryFinallyStatement under the snapshots for the noDeadCode rule

@leops leops temporarily deployed to aws July 5, 2022 13:24 Inactive
@github-actions
Copy link

github-actions bot commented Jul 5, 2022

@github-actions
Copy link

github-actions bot commented Jul 5, 2022

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45878 45878 0
Passed 44938 44938 0
Failed 940 940 0
Panics 0 0 0
Coverage 97.95% 97.95% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 39 39 0
Passed 36 36 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.31% 92.31% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 5946 5946 0
Passed 388 388 0
Failed 5558 5558 0
Panics 0 0 0
Coverage 6.53% 6.53% 0.00%

ts/babel

Test result main count This PR count Difference
Total 588 588 0
Passed 519 519 0
Failed 69 69 0
Panics 0 0 0
Coverage 88.27% 88.27% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16257 16257 0
Passed 12393 12393 0
Failed 3864 3864 0
Panics 0 0 0
Coverage 76.23% 76.23% 0.00%

self.append_instruction(InstructionKind::Jump {
conditional,
block,
finally_fallthrough: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if it's proper to put a language-specific property into "rome_control_flow", which seems to be a language-irrelevant crate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

finally clauses are a concept found in a few other languages than JavaScript (like Java, C#, Python, ...), languages that don't use it simply won't emit instructions with this flag (I specifically made it possible to create an instruction with this flag through the specialized append_finally_fallthrough method so it doesn't pollute all the other call sites of append_jump and languages that don't use it can transparently ignore it)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for explaining

@ematipico ematipico merged commit d95e02c into main Jul 7, 2022
@ematipico ematipico deleted the feature/exception-control-flow branch July 7, 2022 06:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants