Follow MRI behavior for Hash#{each,each_pair}

PullRequest: truffleruby/721

Author: eregon

CHANGELOG.md

@@ -20,6 +20,11 @@ Bug fixes:
   to assign the iteration variable.
 * Existing global variables can now become aliases of other global variables (#1590).
 
+Compatibility:
+
+* Yield different number of arguments for `Hash#each` and `Hash#each_pair` based
+  on the block arity like MRI (#1629).
+
 # 1.0 RC 14
 
 Updated to Ruby 2.6.2.

spec/ruby/core/file/mtime_spec.rb

@@ -12,7 +12,7 @@
 
   it "returns the modification Time of the file" do
     File.mtime(@filename).should be_kind_of(Time)
-    File.mtime(@filename).should be_close(@mtime, 2.0)
+    File.mtime(@filename).should be_close(@mtime, 60.0)
   end
 
   guard -> { platform_is :linux or (platform_is :windows and ruby_version_is '2.5') } do

spec/ruby/core/hash/shared/each.rb

@@ -1,4 +1,6 @@
 describe :hash_each, shared: true do
+
+  # This is inconsistent with below, MRI checks the block arity in rb_hash_each_pair()
   it "yields a [[key, value]] Array for each pair to a block expecting |*args|" do
     all_args = []
     { 1 => 2, 3 => 4 }.send(@method) { |*args| all_args << args }
@@ -19,6 +21,21 @@
     ary.sort.should == ["a", "b", "c"]
   end
 
+  it "yields 2 values and not an Array of 2 elements" do
+    obj = Object.new
+    def obj.foo(key, value)
+      ScratchPad << key << value
+    end
+
+    ScratchPad.record([])
+    { "a" => 1 }.each(&obj.method(:foo))
+    ScratchPad.recorded.should == ["a", 1]
+
+    ScratchPad.record([])
+    { "a" => 1 }.each(&-> key, value { ScratchPad << key << value })
+    ScratchPad.recorded.should == ["a", 1]
+  end
+
   it "uses the same order as keys() and values()" do
     h = { a: 1, b: 2, c: 3, d: 5 }
     keys = []

src/main/java/org/truffleruby/core/hash/HashNodes.java

@@ -427,6 +427,8 @@ protected boolean equalKeys(boolean compareByIdentity, Object key, int hashed, O
     @ImportStatic(HashGuards.class)
     public abstract static class EachNode extends YieldingCoreMethodNode {
 
+        private final ConditionProfile arityMoreThanOne = ConditionProfile.createBinaryProfile();
+
         @Specialization(guards = "isNullHash(hash)")
         public DynamicObject eachNull(DynamicObject hash, DynamicObject block) {
             return hash;
@@ -475,7 +477,12 @@ public DynamicObject eachBuckets(DynamicObject hash, DynamicObject block) {
         }
 
         private Object yieldPair(DynamicObject block, Object key, Object value) {
-            return yield(block, createArray(new Object[]{key, value}, 2));
+            // MRI behavior, see rb_hash_each_pair()
+            if (arityMoreThanOne.profile(Layouts.PROC.getSharedMethodInfo(block).getArity().getArityNumber() > 1)) {
+                return yield(block, key, value);
+            } else {
+                return yield(block, createArray(new Object[]{key, value}, 2));
+            }
         }
 
     }

src/main/java/org/truffleruby/language/methods/Arity.java

@@ -28,6 +28,7 @@ public class Arity {
     private final int postRequired;
     private final boolean hasKeywordsRest;
     private final String[] keywordArguments;
+    private final int arityNumber;
 
     public Arity(int preRequired, int optional, boolean hasRest) {
         this(preRequired, optional, hasRest, 0, NO_KEYWORDS, false);
@@ -41,6 +42,7 @@ public Arity(int preRequired, int optional, boolean hasRest, int postRequired, S
         this.postRequired = postRequired;
         this.keywordArguments = keywordArguments;
         this.hasKeywordsRest = hasKeywordsRest;
+        this.arityNumber = computeArityNumber();
 
         assert keywordArguments != null && preRequired >= 0 && optional >= 0 && postRequired >= 0 : toString();
     }
@@ -77,7 +79,7 @@ public boolean hasKeywordsRest() {
         return hasKeywordsRest;
     }
 
-    public int getArityNumber() {
+    private int computeArityNumber() {
         int count = getRequired();
 
         if (acceptsKeywords()) {
@@ -91,6 +93,10 @@ public int getArityNumber() {
         return count;
     }
 
+    public int getArityNumber() {
+        return arityNumber;
+    }
+
     public String[] getKeywordArguments() {
         return keywordArguments;
     }