From a2c656195f33fed39d79299fa0d2d7f7bbd724cb Mon Sep 17 00:00:00 2001 From: Sebastien Le Callonnec Date: Sun, 9 Sep 2012 14:23:18 +0100 Subject: [PATCH] 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 --- ...ield_splat_behaviour_causes_pp_to_break.rb | 55 +++++++++++++++++++ src/org/jruby/ast/Yield19Node.java | 5 +- .../javasupport/util/RuntimeHelpers.java | 3 +- test/externals/ruby1.9/test_pp.rb | 6 ++ 4 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 spec/regression/GH-276_yield_splat_behaviour_causes_pp_to_break.rb diff --git a/spec/regression/GH-276_yield_splat_behaviour_causes_pp_to_break.rb b/spec/regression/GH-276_yield_splat_behaviour_causes_pp_to_break.rb new file mode 100644 index 00000000000..e495144c5f4 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/src/org/jruby/ast/Yield19Node.java b/src/org/jruby/ast/Yield19Node.java index 32c120290ba..fb00b6a187c 100644 --- a/src/org/jruby/ast/Yield19Node.java +++ b/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); } } diff --git a/src/org/jruby/javasupport/util/RuntimeHelpers.java b/src/org/jruby/javasupport/util/RuntimeHelpers.java index f8dc46d93c8..625db0b95c1 100644 --- a/src/org/jruby/javasupport/util/RuntimeHelpers.java +++ b/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; } } diff --git a/test/externals/ruby1.9/test_pp.rb b/test/externals/ruby1.9/test_pp.rb index fe65287d88b..2a1047ff967 100644 --- a/test/externals/ruby1.9/test_pp.rb +++ b/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)