Skip to content

Conversation

rdmarsh2
Copy link
Contributor

Most of this PR is fairly straightforward pairings of input and output nodes, plus the addition of flow steps for Phi nodes. The main subtlety is the way functions are modeled in the call graph - there's now a non-AST control flow node for each function with the parameter declarations and body as children. The reason these aren't AstControlFlowNodes is so that nested functions don't appear in the CFG of their containing function and result in duplicated nodes.

@rdmarsh2 rdmarsh2 requested a review from a team as a code owner May 26, 2022 18:02
@github-actions github-actions bot added the Swift label May 26, 2022
}

override DataFlowCallable getEnclosingCallable() { isParameterOf(result, _) }

Check warning

Code scanning / CodeQL

Using implicit `this`

Use of implicit `this`.
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label May 27, 2022
MathiasVP
MathiasVP previously approved these changes May 27, 2022
Copy link
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

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

Excellent! A couple of non-blocking comments, but otherwise this LGTM.

@MathiasVP MathiasVP force-pushed the rdmarsh2/swift/dataflow-global-flow branch from bc83757 to ef31aec Compare May 30, 2022 11:58
@MathiasVP MathiasVP merged commit 7ca0144 into github:main May 30, 2022
}
}

private predicate localFlowSsaInput(Node nodeFrom, Ssa::Definition def, Ssa::Definition next) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this predicate deserves a comment, probably just the same as for the Ruby version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-change-note-required This PR does not need a change note Swift
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants