Skip to content

Ruby: Include SSA "phi reads" in DataFlow::Node #11517

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

Merged
merged 4 commits into from
Dec 7, 2022

Conversation

aibaars
Copy link
Contributor

@aibaars aibaars commented Dec 1, 2022

In this PR, we add #11198 nodes to the DataFlow::Node type. See also: #10927 (comment)

@github-actions github-actions bot added the Ruby label Dec 1, 2022
SsaInput::variableRead(bb2, i2, _, false)
}

/** Same as `lastRefRedef`, but skips uncertain reads. */
pragma[nomagic]
private predicate lastRefSkipUncertainReads(Definition def, SsaInput::BasicBlock bb, int i) {
private predicate lastRefSkipUncertainReadsExt(DefinitionExt def, SsaInput::BasicBlock bb, int i) {

Check warning

Code scanning / CodeQL

Missing QLDoc for parameter

The QLDoc has no documentation for bb, or def, or i, but the QLDoc mentions lastRefRedef
@aibaars aibaars force-pushed the phi-reads-in-data-flow-graph branch 4 times, most recently from ea99167 to 85a726b Compare December 2, 2022 10:25
@aibaars aibaars added the no-change-note-required This PR does not need a change note label Dec 2, 2022
@aibaars aibaars marked this pull request as ready for review December 2, 2022 13:45
@aibaars aibaars requested a review from a team as a code owner December 2, 2022 13:45
@calumgrant calumgrant requested a review from hvitved December 5, 2022 09:22
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

LGTM, just one minor thing. And as discussed offline, we may also want to add a stress test like for C#. Lastly, we should wait for DCA to work before merging.

@aibaars aibaars force-pushed the phi-reads-in-data-flow-graph branch from 7c21758 to 7f2ba7d Compare December 5, 2022 20:19
@aibaars aibaars force-pushed the phi-reads-in-data-flow-graph branch from 7f2ba7d to d862972 Compare December 7, 2022 14:34
@hvitved hvitved merged commit 3593806 into github:main Dec 7, 2022
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 Ruby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants