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

feat(engine) Rewrite control flow inside loops. #988

Merged
merged 13 commits into from
Oct 24, 2024
Merged

Conversation

maximebuyse
Copy link
Contributor

@maximebuyse maximebuyse commented Oct 10, 2024

Closes #393.

Loops are already translated as folds. This PR wraps their accumulators to add control flow information (in case of break return or continue). Some specific fold operators are also added to handle these wrapped accumulators.

This PR puts LocalMutation before RewriteControlFlow. In RewriteControlFlow, we now deal with the body of loops (to push control flow in return position, like this phase already does for returns outside loops). We also wrap loops that have a return inside a pattern matching over the result (to propagate a returned value), and apply the phase to inline what comes after in the branches of the pattern matching. Phase DropNeedlessReturns is now DropREturnBreakContinue. Inside loops it replaces return , break and continue by their encoding in the ControlFlow enum. It treats a value in return position of a loop body like a continue. Phase FunctionalizeLoops selects the correct fold operator (each fold should now come in 3 versions: the normal one, the one supporting break and continue, and the one supporting all type of control flow).

@maximebuyse maximebuyse self-assigned this Oct 10, 2024
@maximebuyse
Copy link
Contributor Author

Code generation should be almost stable. The remaining part is to write the specific folds as part of the fstar Rust_primitives

@maximebuyse maximebuyse force-pushed the return-break-continue branch 3 times, most recently from 0233971 to 5fd14ea Compare October 21, 2024 14:13
@maximebuyse
Copy link
Contributor Author

@W95Psp still testing but you can start to review.

@maximebuyse maximebuyse marked this pull request as ready for review October 22, 2024 08:54
@maximebuyse maximebuyse changed the title Rewrite control flow inside loops. feat(engine) Rewrite control flow inside loops. Oct 22, 2024
Copy link
Collaborator

@W95Psp W95Psp left a comment

Choose a reason for hiding this comment

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

Thanks! I have a first set of comments!

engine/lib/ast.ml Outdated Show resolved Hide resolved
engine/lib/ast.ml Outdated Show resolved Hide resolved
engine/lib/ast_utils.ml Outdated Show resolved Hide resolved
engine/lib/phases/phase_drop_return_break_continue.ml Outdated Show resolved Hide resolved
engine/lib/phases/phase_drop_return_break_continue.ml Outdated Show resolved Hide resolved
engine/lib/phases/phase_functionalize_loops.ml Outdated Show resolved Hide resolved
engine/lib/phases/phase_functionalize_loops.ml Outdated Show resolved Hide resolved
engine/lib/phases/phase_reject.ml Outdated Show resolved Hide resolved
engine/lib/phases/phase_rewrite_control_flow.ml Outdated Show resolved Hide resolved
engine/lib/phases/phase_rewrite_control_flow.ml Outdated Show resolved Hide resolved
@maximebuyse
Copy link
Contributor Author

@W95Psp I think I fixed everything according to your feedback!

@W95Psp
Copy link
Collaborator

W95Psp commented Oct 24, 2024

Great, looking at it. Next PR can you resolve discussions that you think are handled?

engine/lib/ast_builder.ml Outdated Show resolved Hide resolved
engine/lib/ast_builder.ml Show resolved Hide resolved
engine/lib/ast_builder.ml Show resolved Hide resolved
@W95Psp W95Psp enabled auto-merge October 24, 2024 08:34
@W95Psp W95Psp added this pull request to the merge queue Oct 24, 2024
Merged via the queue into main with commit f5550da Oct 24, 2024
13 checks passed
@W95Psp W95Psp deleted the return-break-continue branch October 24, 2024 09:24
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.

Add a RewriteControlFlow phase (return/continue/break)
2 participants