Skip to content

Commit

Permalink
Fix for issue jruby#276
Browse files Browse the repository at this point in the history
This commit fixes issue jruby#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
  • Loading branch information
tychobrailleur committed Sep 9, 2012
1 parent fc44a6b commit a2c6561
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 4 deletions.
55 changes: 55 additions & 0 deletions 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
5 changes: 3 additions & 2 deletions src/org/jruby/ast/Yield19Node.java
Expand Up @@ -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
Expand All @@ -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);
}
}
3 changes: 1 addition & 2 deletions src/org/jruby/javasupport/util/RuntimeHelpers.java
Expand Up @@ -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)) {

This comment has been minimized.

Copy link
@tychobrailleur

tychobrailleur Sep 9, 2012

Author Owner

This restores the behaviour prior to JRUBY-6729

argsResult = newResult;
}
}
Expand Down
6 changes: 6 additions & 0 deletions test/externals/ruby1.9/test_pp.rb
Expand Up @@ -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)
Expand Down

0 comments on commit a2c6561

Please sign in to comment.