Fix evaluation order issue in f(**h, &h.delete(key))

[jeremyevans]

Previously, this would delete the key in h before keyword splatting h. This goes against how ruby handles f(*a, &a.pop) and similar expressions.

Fix this by having the compiler check whether the block pass expression is safe. If it is not safe, then dup the keyword splatted hash before evaluating the block pass expression.

For expression: h=nil; f(**h, &h.delete(:key))

VM instructions before:

0000 putnil                                                           (   1)[Li]
0001 setlocal_WC_0                          h@0
0003 putself
0004 getlocal_WC_0                          h@0
0006 getlocal_WC_0                          h@0
0008 putobject                              :key
0010 opt_send_without_block                 <calldata!mid:delete, argc:1, ARGS_SIMPLE>
0012 splatkw
0013 send                                   <calldata!mid:f, argc:1, ARGS_BLOCKARG|FCALL|KW_SPLAT>, nil
0016 leave

VM instructions after:

0000 putnil                                                           (   1)[Li]
0001 setlocal_WC_0                          h@0
0003 putself
0004 putspecialobject                       1
0006 newhash                                0
0008 getlocal_WC_0                          h@0
0010 opt_send_without_block                 <calldata!mid:core#hash_merge_kwd, argc:2, ARGS_SIMPLE>
0012 getlocal_WC_0                          h@0
0014 putobject                              :key
0016 opt_send_without_block                 <calldata!mid:delete, argc:1, ARGS_SIMPLE>
0018 send                                   <calldata!mid:f, argc:1, ARGS_BLOCKARG|FCALL|KW_SPLAT|KW_SPLAT_MUT>, nil
0021 leave

Fixes [Bug #20640]

[github-actions[bot]]

Try on Playground: https://ruby.github.io/play-ruby?run=10801437377 This is an automated comment by pr-playground.yml workflow.

[jeremyevans]

@kddnewton This is my first time contributing to the prism compiler, can you please review that part?

[kddnewton]

Hey @jeremyevans — yes sorry I've been working on a bunch of stuff this week so didn't get the time. I'll look at this first time next week.

[jeremyevans]

No rush on this, early next week is fine. Congratulations on getting prism merged!

[jeremyevans]

Already CI failures in the allocation tests due to this. Going to revert. Not sure why all tests passed in the PR before merge.

[jeremyevans]

Not sure why allocation tests passed in this PR. I can only guess the allocation tests weren't run for prism at the time of the CI.

I submitted a pull request that fixed the allocation tests and still passed the added test (https://github.com/ruby/ruby/pull/11645), but it isn't correct as it is causing segfaults in other cases.