-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Swift: Update SSA to reference the new use-use predicates. #19177
Conversation
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.
Copilot wasn't able to review any files in this pull request.
Files not reviewed (2)
- swift/ql/lib/codeql/swift/dataflow/Ssa.qll: Language not supported
- swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll: Language not supported
Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more
It appears that the changes do improve flow slightly - enough to fix a missing flow in a test. |
DCA is showing two new results (👍) but a slowdown in analysis times, including one project apparently timing out during analysis. |
I don't see this? (neither in the first nor second dca run)
Did you look at the second dca run? I fixed an accidental CP and reran dca, so there shouldn't be any timeouts. |
I also cannot reproduce any of the supposed timing regressions in the second dca run locally, so I'm leaning towards considering those numbers noise. |
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.
Looks good to me. Regarding the DCA slowdown, 1/3 of the projects got faster and the remaining 2/3 got slower. So I agree that it is very likely just noise.
Something about the use-use steps in data flow looks buggy, but I'm not going to fix that here. This is just aiming for a one-for-one semantically equivalent switch to the new use-use predicates.