-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Ssa: Refactor the data flow integration module #18857
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
Conversation
This is a result of the commit "SSA: Fix bug in guards for ssa input nodes."
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 reviewed 13 out of 13 changed files in this pull request and generated no comments.
Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more
5e2c8f2 to
449150e
Compare
asgerf
left a comment
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.
JS changes LGTM
hvitved
left a comment
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.
LGTM, only one question.
| */ | ||
| class SsaInputNode extends SsaNode { | ||
| override SsaImpl::DataFlowIntegration::SsaInputNode node; | ||
| SsaImpl::Definition getDefinition() { result = node.getDefinition() } |
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.
Could you change the return type to Ssa::Definition and avoid the infix cast below?
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.
Yep, done.
e2c183d to
28e9644
Compare
Commit-by-commit review encouraged. I've attempted to make every commit self-contained.
This PR includes a sequence of small changes, including: