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

improvement(semantic/cfg): better break and continue flow. #3462

Conversation

rzvxa
Copy link
Collaborator

@rzvxa rzvxa commented May 29, 2024

This PR adds a new edge type called Jump to distinguish between normal edges and jumps.
There is also a control flow context which is used to keep track of cfg scopes and labels. It replaces the old preserve_state and restore_state.
It corrects some mistakes - such as labeled blocks especially labeled continue which wasn't easy to implement with the old approach - in the old control flow but other than that it is mostly refactored to have a more declarative API instead of a procedural approach.

Copy link

graphite-app bot commented May 29, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions github-actions bot added A-linter Area - Linter A-semantic Area - Semantic labels May 29, 2024
Copy link

codspeed-hq bot commented May 29, 2024

CodSpeed Performance Report

Merging #3462 will not alter performance

Comparing 05-29-improvement_semantic_cfg_better_break_and_continue_flow (3c7ee85) with main (0007ee4)

Summary

✅ 22 untouched benchmarks

@rzvxa rzvxa marked this pull request as ready for review May 30, 2024 20:44
@rzvxa rzvxa marked this pull request as draft May 30, 2024 20:49
@rzvxa rzvxa marked this pull request as ready for review June 3, 2024 18:36
@rzvxa rzvxa force-pushed the 05-28-improvement_semantic_cfg_better_control_flow_for_forstatement_s branch from 857840a to 3482d50 Compare June 5, 2024 19:14
@rzvxa rzvxa force-pushed the 05-29-improvement_semantic_cfg_better_break_and_continue_flow branch from 968a4af to 7d75efc Compare June 5, 2024 19:14
@rzvxa rzvxa force-pushed the 05-28-improvement_semantic_cfg_better_control_flow_for_forstatement_s branch from 3482d50 to c71f0fa Compare June 5, 2024 19:30
@rzvxa rzvxa force-pushed the 05-29-improvement_semantic_cfg_better_break_and_continue_flow branch from 7d75efc to f22307a Compare June 5, 2024 19:30
@Boshen Boshen force-pushed the 05-28-improvement_semantic_cfg_better_control_flow_for_forstatement_s branch from c71f0fa to ff3f37d Compare June 6, 2024 05:42
@Boshen Boshen changed the base branch from 05-28-improvement_semantic_cfg_better_control_flow_for_forstatement_s to main June 6, 2024 05:48
@Boshen
Copy link
Member

Boshen commented Jun 6, 2024

I sometimes don't understand how graphite works, why is conflicting on everything 🤔

@rzvxa rzvxa force-pushed the 05-29-improvement_semantic_cfg_better_break_and_continue_flow branch from f22307a to f4cd08a Compare June 6, 2024 06:04
@rzvxa
Copy link
Collaborator Author

rzvxa commented Jun 6, 2024

I sometimes don't understand how graphite works, why is conflicting on everything 🤔

I don't know why it was conflicting either, I've restacked it and it went without any conflicts. It might be the rerere that took care of that, I'm not sure.

@rzvxa
Copy link
Collaborator Author

rzvxa commented Jun 6, 2024

Nope, seems like there is still something wrong, I'll fix it and let you know.

@rzvxa rzvxa force-pushed the 05-29-improvement_semantic_cfg_better_break_and_continue_flow branch from 4821d02 to 56baa5d Compare June 6, 2024 06:16
@rzvxa
Copy link
Collaborator Author

rzvxa commented Jun 6, 2024

@Boshen OK, I can confirm it was the rerere that resolved the conflict, and the error mentioned in the previous comment was just my old mistake getting auto-repeated. Now it should be good to go.

Copy link

graphite-app bot commented Jun 6, 2024

Merge activity

This PR adds a new edge type called `Jump` to distinguish between normal edges and jumps.
There is also a control flow context which is used to keep track of cfg scopes and labels. It replaces the old `preserve_state` and `restore_state`.
It corrects some mistakes - such as labeled blocks especially labeled continue which wasn't easy to implement with the old approach - in the old control flow but other than that it is mostly refactored to have a more declarative API instead of a procedural approach.
@Boshen Boshen force-pushed the 05-29-improvement_semantic_cfg_better_break_and_continue_flow branch from 56baa5d to 3c7ee85 Compare June 6, 2024 07:56
@graphite-app graphite-app bot merged commit 3c7ee85 into main Jun 6, 2024
22 checks passed
@graphite-app graphite-app bot deleted the 05-29-improvement_semantic_cfg_better_break_and_continue_flow branch June 6, 2024 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter A-semantic Area - Semantic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants