-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Rust: Include patterns as data flow nodes #17971
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
Conversation
| } | ||
|
|
||
| /** A data flow node that corresponds to a CFG node for an AST node. */ | ||
| abstract private class AstCfgFlowNode extends Node { |
Check warning
Code scanning / CodeQL
UnusedField Warning
field n
ParameterNode
This class declares the
field n
ParameterNode
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.
Not sure if this is a real problem. n is used in one of the predicates in AstCfgFlowNode and the subclasses do bind it.
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.
You may be able to suppress it with something like
bindingset[this, n]
AstCfgFlowNode() { exists(this) and exists(n) }
hvitved
left a comment
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.
LGTM.
| } | ||
|
|
||
| /** A data flow node that corresponds to a CFG node for an AST node. */ | ||
| abstract private class AstCfgFlowNode extends Node { |
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.
You may be able to suppress it with something like
bindingset[this, n]
AstCfgFlowNode() { exists(this) and exists(n) }| exists(LetStmt s | | ||
| nodeFrom.getCfgNode().getAstNode() = s.getInitializer() and | ||
| nodeTo.getCfgNode().getAstNode() = s.getPat() | ||
| ) |
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.
This does not take CFG splitting into account, but that changes once #17918 lands.
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. I can also adapt this after #17918 (if you don't want to do it all yourself).
Adds patterns as data flow nodes.
Also adds flow from the rhs. of a
letstatement to the lhs. which depends on patterns being in the data flow nodes. That will need some tweaking once we have the CFG layer over the AST layer.Adds an abstract class for data flow node that corresponds to a CFG nodes for a AST nodes to factor out some common code.