Bug #10557: Block not given when the argument is a string

• Status: Closed
• Assignee: ko1 (Koichi Sasada)
• Target version: 2.2.0
• ruby -v: ruby 2.2.0dev (2014-11-30 trunk 48655) [x86_64-darwin14]
• Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN

[bartoszkopinski (Bartosz Kopinski)]

This seems really weird and random. Can anyone else confirm it's bug?

class Klass
  def [](_)
    block_given?
  end
end

# ruby 2.1.5p273 (2014-11-13 revision 48405) [x86_64-darwin14.0]
Klass.new.[](nil){ }    # => true
Klass.new.[](0){ }      # => true
Klass.new.[](false){ }  # => true
Klass.new.[](''){ }     # => true

# ruby 2.2.0dev (2014-11-30 trunk 48655) [x86_64-darwin14]
Klass.new.[](nil){ }    # => true
Klass.new.[](0){ }      # => true
Klass.new.[](false){ }  # => true
Klass.new.[](''){ }     # => false

[marcandre (Marc-Andre Lafortune)]

• Category set to core
• Priority changed from Normal to 6

Ouch.

[ko1 (Koichi Sasada)]

• Assignee set to ko1 (Koichi Sasada)

This is an issue in compile.c (opt_aref_with). There is same issue on opt_aset_with, opt_str_freeze.

[normalperson (Eric Wong)]

I think the following should fix it, OK to commit?

 * compile.c (iseq_compile_each): only emit opt_str_freeze,
   opt_aref_with, and opt_aset_with insn when no block is given
   [Bug #10557] [ruby-core:66595]
 * test/ruby/test_optimization.rb (test_block_given_aset_aref):
   new test for bug thanks to Bartosz Kopinski.
   (test_string_freeze): additional assertion for object_id
 ---
  compile.c                      |  9 ++++++---
  test/ruby/test_optimization.rb | 36 ++++++++++++++++++++++++++++++++++++
  2 files changed, 42 insertions(+), 3 deletions(-)
 
 diff --git a/compile.c b/compile.c
 index 6c6d933..8672c0e 100644
 --- a/compile.c
 +++ b/compile.c
 @@ -4405,7 +4405,8 @@ iseq_compile_each(rb_iseq_t *iseq, LINK_ANCHOR *ret, NODE * node, int poped)
  	 *   "literal".freeze -> opt_str_freeze("literal")
  	 */
  	if (node->nd_recv && nd_type(node->nd_recv) == NODE_STR &&
 -	    node->nd_mid == idFreeze && node->nd_args == NULL)
 +	    node->nd_mid == idFreeze && node->nd_args == NULL &&
 +	    iseq->compile_data->current_block == Qfalse)
  	{
  	    VALUE str = rb_fstring(node->nd_recv->nd_lit);
  	    iseq_add_mark_object(iseq, str);
 @@ -4420,7 +4421,8 @@ iseq_compile_each(rb_iseq_t *iseq, LINK_ANCHOR *ret, NODE * node, int poped)
  	 */
  	if (node->nd_mid == idAREF && !private_recv_p(node) && node->nd_args &&
  	    nd_type(node->nd_args) == NODE_ARRAY && node->nd_args->nd_alen == 1 &&
 -	    nd_type(node->nd_args->nd_head) == NODE_STR)
 +	    nd_type(node->nd_args->nd_head) == NODE_STR &&
 +	    iseq->compile_data->current_block == Qfalse)
  	{
  	    VALUE str = rb_fstring(node->nd_args->nd_head->nd_lit);
  	    node->nd_args->nd_head->nd_lit = str;
 @@ -5416,7 +5418,8 @@ iseq_compile_each(rb_iseq_t *iseq, LINK_ANCHOR *ret, NODE * node, int poped)
  	 */
  	if (node->nd_mid == idASET && !private_recv_p(node) && node->nd_args &&
  	    nd_type(node->nd_args) == NODE_ARRAY && node->nd_args->nd_alen == 2 &&
 -	    nd_type(node->nd_args->nd_head) == NODE_STR)
 +	    nd_type(node->nd_args->nd_head) == NODE_STR &&
 +	    iseq->compile_data->current_block == Qfalse)
  	{
  	    VALUE str = rb_fstring(node->nd_args->nd_head->nd_lit);
  	    node->nd_args->nd_head->nd_lit = str;
 diff --git a/test/ruby/test_optimization.rb b/test/ruby/test_optimization.rb
 index 129f62a..4a5484e 100644
 --- a/test/ruby/test_optimization.rb
 +++ b/test/ruby/test_optimization.rb
 @@ -118,6 +118,7 @@ class TestRubyOptimization < Test::Unit::TestCase
  
    def test_string_freeze
      assert_equal "foo", "foo".freeze
 +    assert_equal "foo".freeze.object_id, "foo".freeze.object_id
      assert_redefine_method('String', 'freeze', 'assert_nil "foo".freeze')
    end
  
 @@ -253,4 +254,39 @@ class TestRubyOptimization < Test::Unit::TestCase
      EOF
      assert_equal(123, delay { 123 }.call, bug6901)
    end
 +
 +  class Bug10557
 +    def [](_)
 +      block_given?
 +    end
 +
 +    def []=(_, _)
 +      block_given?
 +    end
 +  end
 +
 +  def test_block_given_aset_aref
 +    bug10557 = '[ruby-core:66595]'
 +    assert_equal(true, Bug10557.new.[](nil){}, bug10557)
 +    assert_equal(true, Bug10557.new.[](0){}, bug10557)
 +    assert_equal(true, Bug10557.new.[](false){}, bug10557)
 +    assert_equal(true, Bug10557.new.[](''){}, bug10557)
 +    assert_equal(true, Bug10557.new.[]=(nil, 1){}, bug10557)
 +    assert_equal(true, Bug10557.new.[]=(0, 1){}, bug10557)
 +    assert_equal(true, Bug10557.new.[]=(false, 1){}, bug10557)
 +    assert_equal(true, Bug10557.new.[]=('', 1){}, bug10557)
 +  end
 +
 +  def test_string_freeze_block
 +    assert_separately([], <<-"end;")#    do
 +      class String
 +        undef freeze
 +        def freeze
 +          block_given?
 +        end
 +      end
 +      assert_equal(true, "block".freeze {})
 +      assert_equal(false, "block".freeze)
 +    end;
 +  end
  end

--
EW

[ko1 (Koichi Sasada)]

On 2014/12/17 6:12, Eric Wong wrote:

I think the following should fix it, OK to commit?

OK. Thank you.

--
// SASADA Koichi at atdot dot net

• Status changed from Open to Closed
• % Done changed from 0 to 100

Applied in changeset r48869.

---

compile.c: skip opt_* insns for fstr args with block given

• compile.c (iseq_compile_each): only emit opt_str_freeze,
  opt_aref_with, and opt_aset_with insn when no block is given
  [Bug #10557] [ruby-core:66595]
• test/ruby/test_optimization.rb (test_block_given_aset_aref):
  new test for bug thanks to Bartosz Kopinski.
  (test_string_freeze): additional assertion for object_id