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

Rust: Add local data flow edge for SSA nodes #18026

Merged
merged 4 commits into from
Nov 19, 2024

Conversation

paldepind
Copy link
Contributor

Adds some additional local data flow tests and fixes a few missing results by adding an edge from patterns to their corresponding SSA node.

I'm not 100% that this is the correct thing to do in all cases (complex patterns) but it did fix the tests, and we can iterate on this.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Nov 19, 2024
1000 + i
}

fn sink(s: i64) {

Check notice

Code scanning / CodeQL

Unused variable Note test

Variable 's' is not used.
(
LocalFlow::localFlowStepCommon(nodeFrom, nodeTo)
or
SsaFlow::localFlowStep(_, nodeFrom, nodeTo, _)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be part of localFlowStepCommon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've moved it. I copied the pattern from Ruby where it's also structured st. the SSA flow is not in localFlowStepCommon.

Comment on lines 286 to 288
// An edge from a pattern to its corresponding SSA definition.
nodeFrom.(Node::PatNode).getPat() =
nodeTo.(Node::SsaNode).getDefinitionExt().getSourceVariable().getPat()
Copy link
Contributor

Choose a reason for hiding this comment

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

Should rather be:

    // An edge from a pattern/expression to its corresponding SSA definition.
    nodeFrom.(Node::AstCfgFlowNode).getCfgNode() =
      nodeTo.(Node::SsaNode).getDefinitionExt().(Ssa::WriteDefinition).getControlFlowNode()

Copy link
Contributor Author

@paldepind paldepind Nov 19, 2024

Choose a reason for hiding this comment

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

Thanks 🙇. That makes sense.

@@ -112,7 +112,7 @@
}

/** A data flow node that corresponds to a CFG node for an AST node. */
abstract private class AstCfgFlowNode extends Node {
abstract class AstCfgFlowNode extends Node {

Check warning

Code scanning / CodeQL

UnusedField Warning

This class declares the
field n
but does not bind it in the characteristic predicate of any class between it and
ParameterNode
.
This class declares the
field n
but does not bind it in the characteristic predicate of any class between it and
ParameterNode
.
@paldepind paldepind merged commit e595151 into github:main Nov 19, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants