Skip to content

Ruby: Fix bad join in DeadStoreOfLocal.ql #19259

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 1 commit into from
Apr 9, 2025

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Apr 9, 2025

[2025-04-09 09:22:56] (380s) Cancelling evaluation of #11561 evaluator _ApiGraphs::API::Impl::MkMethodAccessNode#4daa063f_ApiGraphs::API::Impl::TApiNode#368fc76d_ApiGraphs__#antijoin_rhs/1@d02e8ckf: Demand has vanished for #11561 evaluator _ApiGraphs::API::Impl::MkMethodAccessNode#4daa063f_ApiGraphs::API::Impl::TApiNode#368fc76d_ApiGraphs__#antijoin_rhs/1@d02e8ckf
[2025-04-09 09:22:56] (380s) Tuple counts for _ApiGraphs::API::Impl::MkMethodAccessNode#4daa063f_ApiGraphs::API::Impl::TApiNode#368fc76d_ApiGraphs__#antijoin_rhs/1@d02e8ckf after 5m17s:
                      12500       ~2341%      {2} r1 = JOIN `__Variable::LocalVariable.getDefiningAccess/0#dispred#316bde3e_10#join_rhs__Variable::LocalVariableW__#shared` WITH `Statement::Stmt.getCfgScope/0#dispred#58b7c05d` ON FIRST 1 OUTPUT Rhs.1, Lhs.0 'arg0'
                      1700171     ~775%       {2}    | JOIN WITH `ControlFlowGraphImpl::NodeImpl.getScope/0#dispred#fd525252_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1 'arg0'
                      1530333     ~722%       {3}    | JOIN WITH cached_DataFlowPrivate::TExprNode#ec061b78 ON FIRST 1 OUTPUT _, Lhs.1 'arg0', Rhs.1
                      1530333     ~699%       {3}    | REWRITE WITH Out.0 := "ERB"
                      258054437   ~741%       {3}    | JOIN WITH `cached_ApiGraphs::API::Impl::topLevelMember/2#ce51cf11` ON FIRST 1 OUTPUT Rhs.1, Lhs.1 'arg0', Lhs.2
                      258054437   ~741%       {3}    | JOIN WITH cached_ApiGraphs::API::Impl::TApiNode#368fc76d ON FIRST 1 OUTPUT Lhs.0, Lhs.1 'arg0', Lhs.2
                      832733500   ~1726%      {3}    | JOIN WITH `cached_fastTC@ApiGraphs::API::Internal::MkShared::Cached::epsilonEdge/2#aec408a8#2` ON FIRST 1 OUTPUT Rhs.1, Lhs.1 'arg0', Lhs.2
                      450751669   ~1518%      {3}    | JOIN WITH `cached_ApiGraphs::API::Impl::instanceEdge/2#f60af161` ON FIRST 1 OUTPUT Rhs.1, Lhs.1 'arg0', Lhs.2
                      36309175000 ~11844%     {4}    | JOIN WITH `cached_fastTC@ApiGraphs::API::Internal::MkShared::Cached::epsilonEdge/2#aec408a8#2` ON FIRST 1 OUTPUT Rhs.1, _, Lhs.1 'arg0', Lhs.2
                      36309173500 ~11194%     {4}    | REWRITE WITH Out.1 := "result"
                      447552127   ~2171%      {3}    | JOIN WITH `cached_ApiGraphs::API::Impl::methodEdge/3#5c5f7602` ON FIRST 2 OUTPUT Lhs.3, Rhs.2, Lhs.2 'arg0'
                      0           ~0%         {1}    | JOIN WITH cached_ApiGraphs::API::Impl::MkMethodAccessNode#4daa063f ON FIRST 2 OUTPUT Lhs.2 'arg0'
                                              return r1

@github-actions github-actions bot added the Ruby label Apr 9, 2025
@hvitved hvitved marked this pull request as ready for review April 9, 2025 07:36
@Copilot Copilot AI review requested due to automatic review settings April 9, 2025 07:36
@hvitved hvitved requested a review from a team as a code owner April 9, 2025 07:36
Copy link
Contributor

@Copilot Copilot AI left a 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 (1)
  • ruby/ql/src/queries/variables/DeadStoreOfLocal.ql: Language not supported

@hvitved hvitved added the no-change-note-required This PR does not need a change note label Apr 9, 2025
@hvitved hvitved requested a review from yoff April 9, 2025 07:36
Copy link
Contributor

@yoff yoff left a comment

Choose a reason for hiding this comment

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

Wow, so it first finds all ERB nodes, then all results of anything and then put the two together...and then joins on scopes.
I read the fix as joining on the scopes first. I wonder if hasErbResultCall still has the funky join behavior in API graph part.

Thanks for the fix! Happy to merge as soon as DCA comes back.

@hvitved hvitved merged commit a6b20d7 into github:main Apr 9, 2025
25 of 26 checks passed
@hvitved hvitved deleted the ruby/fix-bad-join branch April 9, 2025 17:03
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