Skip to content

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.


/** 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