-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Rust: Add {BreakExpr,ContinueExpr}.getTarget()
#17644
Conversation
053e6da
to
5abaea3
Compare
5abaea3
to
6da3972
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and certainly cleaner than the original code, though I forget some of the specifics of how ControlFlowTree
nodes are supposed to be wired. The tests add a lot of confidence.
} | ||
|
||
override predicate last(AstNode last, Completion c) { none() } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is having no last
node valid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and we want it because the edge out of {Break,Continue}Expr
is defined explicitly (as the jump target) in the succ
relation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, having a last
node would cause an (incorrect) edge to be created from the break
to the next statement sequentially in the program.
It is convenient to be able to resolve the target of a
break
orcontinue
expression, without relying on the CFG. This furthermore allows for the CFG construction to be simplified.