From 6f3f1f2348b3a12e2cfeb35b19cf025d22a80a41 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Tue, 27 Feb 2024 16:40:10 -0500 Subject: [PATCH 1/5] arity for {|a,| } was -2 and should have been 1 --- core/src/main/java/org/jruby/runtime/Signature.java | 3 ++- spec/tags/ruby/core/proc/arity_tags.txt | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) delete mode 100644 spec/tags/ruby/core/proc/arity_tags.txt diff --git a/core/src/main/java/org/jruby/runtime/Signature.java b/core/src/main/java/org/jruby/runtime/Signature.java index d4725735b85..f4bf8a6a340 100644 --- a/core/src/main/java/org/jruby/runtime/Signature.java +++ b/core/src/main/java/org/jruby/runtime/Signature.java @@ -135,8 +135,9 @@ public int calculateArityValue() { int oneForKeywords = requiredKwargs > 0 ? 1 : 0; int fixedValue = pre() + post() + oneForKeywords; boolean hasOptionalKeywords = kwargs - requiredKwargs > 0; + boolean optionalFromRest = rest() != Rest.NONE && rest != Rest.ANON; - if (opt() > 0 || rest() != Rest.NONE || (hasOptionalKeywords || restKwargs()) && oneForKeywords == 0) { + if (opt() > 0 || optionalFromRest || (hasOptionalKeywords || restKwargs()) && oneForKeywords == 0) { return -1 * (fixedValue + 1); } diff --git a/spec/tags/ruby/core/proc/arity_tags.txt b/spec/tags/ruby/core/proc/arity_tags.txt deleted file mode 100644 index e742e2ff5e3..00000000000 --- a/spec/tags/ruby/core/proc/arity_tags.txt +++ /dev/null @@ -1,2 +0,0 @@ -fails:Proc#arity for instances created with lambda { || } returns positive values for definition '@a = lambda { |a, | }' -fails:Proc#arity for instances created with proc { || } returns positive values for definition '@a = proc { |a, | }' From 8261c447aa8303a8a6d6900816b6d7d265062a70 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Wed, 28 Feb 2024 09:46:25 -0500 Subject: [PATCH 2/5] After correcting arity I need to account for non-rest still spreading --- core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java | 4 +++- core/src/main/java/org/jruby/runtime/Signature.java | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java index 91e1c4762cf..e02932a7a4c 100644 --- a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java +++ b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java @@ -513,7 +513,9 @@ public static IRubyObject[] convertValueIntoArgArray(ThreadContext context, IRub new IRubyObject[] { value }; case 0: case 1: - return new IRubyObject[] { value }; + return signature.isSpreadable() ? + IRBlockBody.toAry(context, value) : + new IRubyObject[] { value }; } return IRBlockBody.toAry(context, value); diff --git a/core/src/main/java/org/jruby/runtime/Signature.java b/core/src/main/java/org/jruby/runtime/Signature.java index f4bf8a6a340..20fd06cb697 100644 --- a/core/src/main/java/org/jruby/runtime/Signature.java +++ b/core/src/main/java/org/jruby/runtime/Signature.java @@ -154,7 +154,7 @@ public int arityValue() { * @return true if the signature expects multiple args */ public boolean isSpreadable() { - return arityValue < -1 || arityValue > 1 || (opt > 0 && !restKwargs()); + return arityValue < -1 || arityValue > 1 || (opt > 0 && !restKwargs()) || rest == Rest.ANON; } From d815fe182358378fa41b26f9195c0d98b3d1e90a Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Tue, 30 Apr 2024 14:24:12 -0400 Subject: [PATCH 3/5] hmm. arity 1 with anon rest gets us to have more extra logic --- core/src/main/java/org/jruby/RubyHash.java | 2 +- core/src/main/java/org/jruby/runtime/IRBlockBody.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyHash.java b/core/src/main/java/org/jruby/RubyHash.java index 51a29f73ea1..372741193a0 100644 --- a/core/src/main/java/org/jruby/RubyHash.java +++ b/core/src/main/java/org/jruby/RubyHash.java @@ -1624,7 +1624,7 @@ public RubyHash each_pairCommon(final ThreadContext context, final Block block) public void visit(ThreadContext context, RubyHash self, IRubyObject key, IRubyObject value, int index, Block block) { if (block.type == Block.Type.LAMBDA) { block.call(context, context.runtime.newArray(key, value)); - } else if (block.getSignature().arityValue() > 1) { + } else if (block.getSignature().arityValue() > 1 || block.getSignature().isSpreadable()) { block.yieldSpecific(context, key, value); } else { block.yield(context, context.runtime.newArray(key, value)); diff --git a/core/src/main/java/org/jruby/runtime/IRBlockBody.java b/core/src/main/java/org/jruby/runtime/IRBlockBody.java index 6ba10157600..eef96ab07fe 100644 --- a/core/src/main/java/org/jruby/runtime/IRBlockBody.java +++ b/core/src/main/java/org/jruby/runtime/IRBlockBody.java @@ -103,7 +103,7 @@ public IRubyObject yieldSpecific(ThreadContext context, Block block, IRubyObject private IRubyObject yieldSpecificMultiArgsCommon(ThreadContext context, Block block, IRubyObject... args) { int blockArity = signature.arityValue(); - if (blockArity == 1) { + if (blockArity == 1 && !signature.isSpreadable()) { args = new IRubyObject[] { RubyArray.newArrayMayCopy(context.runtime, args) }; } From 92b1d86718af9196f369307df97462a36ed7666a Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Tue, 30 Apr 2024 15:15:01 -0400 Subject: [PATCH 4/5] ACP needs to know about spreadable + arity of 1 --- .../java/org/jruby/ir/passes/AddCallProtocolInstructions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java b/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java index a8a5ab99624..9665ddf08b3 100644 --- a/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java +++ b/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java @@ -121,7 +121,7 @@ public Object execute(FullInterpreterContext fic, Object... data) { Signature sig = ((IRClosure)fic.getScope()).getSignature(); if (sig.isNoArguments()) { prologueBB.addInstr(PrepareNoBlockArgsInstr.INSTANCE); - } else if (sig.isOneArgument() && !sig.hasKwargs()) { // no kwargs and just a single required argument + } else if (sig.isOneArgument() && !sig.isSpreadable() && !sig.hasKwargs()) { // no kwargs and just a single required argument prologueBB.addInstr(PrepareSingleBlockArgInstr.INSTANCE); } else { prologueBB.addInstr(PrepareBlockArgsInstr.INSTANCE); From 16be7285ab4ab07a287774990d230ba557fcb2e2 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Tue, 30 Apr 2024 16:18:59 -0400 Subject: [PATCH 5/5] Some reduction is redundant checks --- core/src/main/java/org/jruby/RubyHash.java | 2 +- .../org/jruby/ir/passes/AddCallProtocolInstructions.java | 2 +- .../src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java | 2 +- core/src/main/java/org/jruby/runtime/IRBlockBody.java | 5 ++--- core/src/main/java/org/jruby/runtime/Signature.java | 2 +- 5 files changed, 6 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyHash.java b/core/src/main/java/org/jruby/RubyHash.java index 372741193a0..8c6e307f149 100644 --- a/core/src/main/java/org/jruby/RubyHash.java +++ b/core/src/main/java/org/jruby/RubyHash.java @@ -1624,7 +1624,7 @@ public RubyHash each_pairCommon(final ThreadContext context, final Block block) public void visit(ThreadContext context, RubyHash self, IRubyObject key, IRubyObject value, int index, Block block) { if (block.type == Block.Type.LAMBDA) { block.call(context, context.runtime.newArray(key, value)); - } else if (block.getSignature().arityValue() > 1 || block.getSignature().isSpreadable()) { + } else if (block.getSignature().isSpreadable()) { block.yieldSpecific(context, key, value); } else { block.yield(context, context.runtime.newArray(key, value)); diff --git a/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java b/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java index 9665ddf08b3..a8a5ab99624 100644 --- a/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java +++ b/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java @@ -121,7 +121,7 @@ public Object execute(FullInterpreterContext fic, Object... data) { Signature sig = ((IRClosure)fic.getScope()).getSignature(); if (sig.isNoArguments()) { prologueBB.addInstr(PrepareNoBlockArgsInstr.INSTANCE); - } else if (sig.isOneArgument() && !sig.isSpreadable() && !sig.hasKwargs()) { // no kwargs and just a single required argument + } else if (sig.isOneArgument() && !sig.hasKwargs()) { // no kwargs and just a single required argument prologueBB.addInstr(PrepareSingleBlockArgInstr.INSTANCE); } else { prologueBB.addInstr(PrepareBlockArgsInstr.INSTANCE); diff --git a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java index e02932a7a4c..95dbcb8b25c 100644 --- a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java +++ b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java @@ -513,7 +513,7 @@ public static IRubyObject[] convertValueIntoArgArray(ThreadContext context, IRub new IRubyObject[] { value }; case 0: case 1: - return signature.isSpreadable() ? + return signature.rest() == org.jruby.runtime.Signature.Rest.ANON ? IRBlockBody.toAry(context, value) : new IRubyObject[] { value }; } diff --git a/core/src/main/java/org/jruby/runtime/IRBlockBody.java b/core/src/main/java/org/jruby/runtime/IRBlockBody.java index eef96ab07fe..ae27eedf281 100644 --- a/core/src/main/java/org/jruby/runtime/IRBlockBody.java +++ b/core/src/main/java/org/jruby/runtime/IRBlockBody.java @@ -102,14 +102,13 @@ public IRubyObject yieldSpecific(ThreadContext context, Block block, IRubyObject } private IRubyObject yieldSpecificMultiArgsCommon(ThreadContext context, Block block, IRubyObject... args) { - int blockArity = signature.arityValue(); - if (blockArity == 1 && !signature.isSpreadable()) { + if (signature.isOneArgument()) { args = new IRubyObject[] { RubyArray.newArrayMayCopy(context.runtime, args) }; } if (canCallDirect()) return yieldDirect(context, block, args, null); - if (blockArity == 0) { + if (signature.arityValue() == 0) { args = IRubyObject.NULL_ARRAY; // discard args } if (block.type == Block.Type.LAMBDA) signature.checkArity(context.runtime, args); diff --git a/core/src/main/java/org/jruby/runtime/Signature.java b/core/src/main/java/org/jruby/runtime/Signature.java index 20fd06cb697..1590067cd66 100644 --- a/core/src/main/java/org/jruby/runtime/Signature.java +++ b/core/src/main/java/org/jruby/runtime/Signature.java @@ -85,7 +85,7 @@ public boolean restKwargs() { * Are there an exact (fixed) number of parameters to this signature? */ public boolean isFixed() { - return arityValue() >= 0; + return arityValue() >= 0 && rest != Rest.ANON; } /**