Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix for issue #276 #293

Closed
wants to merge 1 commit into from

2 participants

@tychobrailleur
Collaborator

This commit fixes issue #276 which is a regression introduced by
http://jira.codehaus.org/browse/JRUBY-6729. The bug was caused by a erroneous
behaviour when yielding splat for a block of arity of one: MRI doesn't unsplat
the passed value, whilst jruby was.

See GH-276_yield_splat_behaviour_causes_pp_to_break.rb for details on behaviour when yielding splat.

@tychobrailleur tychobrailleur Fix for issue #276
This commit fixes issue #276 which is a regression introduced by
http://jira.codehaus.org/browse/JRUBY-6729.  The bug was caused by a erroneous
behavour when yielding splat for a block of arity of one: MRI doesn't unsplat
the passed value, whilst jruby was.

See GH-276_yield_splat_behaviour_causes_pp_to_break.rb for details on behaviour
a2c6561
@tychobrailleur

This restores the behaviour prior to JRUBY-6729

@headius
Owner

A-ha! Very good! I was hoping to find a fix for this issue. I'll have a look and get it merged if it's ok.

@headius headius closed this pull request from a commit
@headius headius Additional fix to go with #276 and its fix #293.
* Compiler needed the same logic.
* Invalid block needs to be checked or arity().getValue will NPE.
da2a9dd
@headius headius closed this in da2a9dd
@prathamesh-sonpatki prathamesh-sonpatki referenced this pull request from a commit in prathamesh-sonpatki/jruby
@headius headius Additional fix to go with #276 and its fix #293.
* Compiler needed the same logic.
* Invalid block needs to be checked or arity().getValue will NPE.
dc19d5d
@prathamesh-sonpatki prathamesh-sonpatki referenced this pull request from a commit in prathamesh-sonpatki/jruby
@headius headius Additional fix to go with #276 and its fix #293.
* Compiler needed the same logic.
* Invalid block needs to be checked or arity().getValue will NPE.
a0441ff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 9, 2012
  1. @tychobrailleur

    Fix for issue #276

    tychobrailleur authored
    This commit fixes issue #276 which is a regression introduced by
    http://jira.codehaus.org/browse/JRUBY-6729.  The bug was caused by a erroneous
    behavour when yielding splat for a block of arity of one: MRI doesn't unsplat
    the passed value, whilst jruby was.
    
    See GH-276_yield_splat_behaviour_causes_pp_to_break.rb for details on behaviour
This page is out of date. Refresh to see the latest.
View
55 spec/regression/GH-276_yield_splat_behaviour_causes_pp_to_break.rb
@@ -0,0 +1,55 @@
+require 'rspec'
+
+def call_one
+ yield(["a"])
+end
+
+def call_two
+ yield(["a","b"])
+end
+
+def call_three
+ yield(["a", "b", "c"])
+end
+
+def yield_with_splat(method_name = 'call_two')
+ send(method_name) { |*a| yield(*a) }
+end
+
+describe 'yield splat' do
+ it 'yields an array when block has only one argument' do
+ value = nil
+ yield_with_splat("call_one") { |a| value = a }
+ value.should == ["a"]
+ end
+
+ it 'yields an array when block as one argument and passed two' do
+ value = nil
+ yield_with_splat("call_two") { |a| value = a }
+ value.should == ["a", "b"]
+ end
+
+ it 'yields one value when block has two arguments and passed one' do
+ first_value = nil
+ second_value = nil
+ yield_with_splat("call_one") { |a,b| first_value = a; second_value = b }
+ first_value.should == "a"
+ second_value.should == nil
+ end
+
+ it 'yields two values when block has two arguments and passed two' do
+ first_value = nil
+ second_value = nil
+ yield_with_splat { |a,b| first_value = a; second_value = b }
+ first_value.should == "a"
+ second_value.should == "b"
+ end
+
+ it 'yields two values when block has two arguments and passed three' do
+ first_value = nil
+ second_value = nil
+ yield_with_splat("call_three") { |a,b| first_value = a; second_value = b }
+ first_value.should == "a"
+ second_value.should == "b"
+ end
+end
View
5 src/org/jruby/ast/Yield19Node.java
@@ -24,12 +24,13 @@ public Yield19Node(ISourcePosition position, Node node) {
public IRubyObject interpret(Ruby runtime, ThreadContext context, IRubyObject self, Block aBlock) {
Node args = getArgsNode();
IRubyObject argsResult = args.interpret(runtime, context, self, aBlock);
+ Block yieldToBlock = context.getCurrentFrame().getBlock();
switch (args.getNodeType()) {
case ARGSPUSHNODE:
case ARGSCATNODE:
case SPLATNODE:
- argsResult = RuntimeHelpers.unsplatValue19(argsResult);
+ if (yieldToBlock.arity().getValue() > 1) argsResult = RuntimeHelpers.unsplatValue19(argsResult);
break;
case ARRAYNODE:
// Pass-thru
@@ -38,6 +39,6 @@ public IRubyObject interpret(Ruby runtime, ThreadContext context, IRubyObject se
assert false: "Invalid node found in yield";
}
- return context.getCurrentFrame().getBlock().yieldArray(context, argsResult, null, null);
+ return yieldToBlock.yieldArray(context, argsResult, null, null);
}
}
View
3  src/org/jruby/javasupport/util/RuntimeHelpers.java
@@ -1820,8 +1820,7 @@ public static IRubyObject unsplatValue19(IRubyObject argsResult) {
if (array.size() == 1) {
IRubyObject newResult = array.eltInternal(0);
- // JRUBY-6729. It seems RubyArray should be returned as it is from here.
- if (!(newResult instanceof RubyArray)) {
+ if (!((newResult instanceof RubyArray) && ((RubyArray) newResult).size() == 0)) {
argsResult = newResult;
}
}
View
6 test/externals/ruby1.9/test_pp.rb
@@ -137,6 +137,12 @@ def test_hash
assert_equal("#{a.inspect}\n", PP.pp(a, ''))
end
+ def test_hash_with_boolean_value
+ a = {}
+ a[:b] = true
+ assert_equal("{:b=>true}\n", PP.pp(a,''))
+ end
+
S = Struct.new("S", :a, :b)
def test_struct
a = S.new(1,2)
Something went wrong with that request. Please try again.