Project

General

Profile

Bug #10557

Block not given when the argument is a string

Added by bartoszkopinski (Bartosz Kopinski) over 5 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Normal
Target version:
ruby -v:
ruby 2.2.0dev (2014-11-30 trunk 48655) [x86_64-darwin14]
[ruby-core:66595]

Description

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

Updated by marcandre (Marc-Andre Lafortune) over 5 years ago

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

Ouch.

Updated by ko1 (Koichi Sasada) over 5 years ago

  • 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.

Updated by normalperson (Eric Wong) over 5 years ago

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

Updated by ko1 (Koichi Sasada) over 5 years ago

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

#5

Updated by Anonymous over 5 years ago

  • 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

Also available in: Atom PDF