Allow Hash#transform_keys to take a hash argument

[ccocchi]

Hello,

This is part of the Ruby 3.0 support - #2453. I've tested the changes against the ruby/spec suite plus the changes I've added to the suite.

About the implementation, I wasn't sure if the check for the mapping presence should be done within the loop (what I ended up doing) or outside of it, which would make the code more verbose/duplicate.

This is my first contribution to truffleruby, so any feedback is welcome 🙂

[graalvmbot]

Hello ccocchi, thanks for contributing a PR to our project!

We use the Oracle Contributor Agreement to make the copyright of contributions clear. We don't have a record of you having signed this yet, based on your email address cocchi -(dot)- c -(at)- gmail -(dot)- com. You can sign it at that link.

If you think you've already signed it, please comment below and we'll check.

[eregon]

Hello, this looks good, thank you for the PR.
Could you sign the OCA (at https://oca.opensource.oracle.com/)?

[graalvmbot]

Hello ccocchi, thanks for contributing a PR to our project!

We use the Oracle Contributor Agreement to make the copyright of contributions clear. We don't have a record of you having signed this yet, based on your email address cocchi -(dot)- c -(at)- gmail -(dot)- com. You can sign it at that link.

If you think you've already signed it, please comment below and we'll check.

[eregon]

Sorry about the duplicate message, it's maybe because I edited the first one to fix the link.

[ccocchi]

Great! I signed it yesterday, it is still under review.

[eregon]

The OCA went through, the bot should react soon.

[eregon]

About the implementation, I wasn't sure if the check for the mapping presence should be done within the loop (what I ended up doing) or outside of it, which would make the code more verbose/duplicate.

I think in this case it would be more readable to have a separate loop, given there are 3 checks for if mapping which could be just 1 if done early. Could you change to that?

Performance-wise, it seems a bit better to have separate loops for the interpreter, and if the method is not split (i.e., the runtime does not create a copy for a call site because e.g. it ran out of split budget).

[graalvmbot]

ccocchi has signed the Oracle Contributor Agreement (based on email address cocchi -(dot)- c -(at)- gmail -(dot)- com) so can contribute to this repository.

[ccocchi]

I think in this case it would be more readable to have a separate loop, given there are 3 checks for if mapping which could be just 1 if done early. Could you change to that?

Changes are done 🙂

Out of curiosity, did you use any tool (like seafoam or other) to determine the performance difference or simply used your overall knowledge about how the interpreter will handle that case?

[eregon]

Out of curiosity, did you use any tool (like seafoam or other) to determine the performance difference or simply used your overall knowledge about how the interpreter will handle that case?

In this case my overall knowledge.

[eregon]

Thanks for the changes, I'll add a CHANGELOG entry and I'll figure out if I add the spec to :next or if I bump the RUBY_VERSION before.