Project

General

Profile

Actions

Bug #17725

open

Prepend breaks ability to override optimized methods

Added by joshuadreed (Josh Reed) 3 months ago. Updated 3 months ago.

Status:
Open
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
ruby 3.0.0p0 (2020-12-25 revision 95aff21468) [x86_64-darwin19]
[ruby-core:102884]

Description

Prepending any module to String and Hash (and possibly other or all classes?) blocks the ability to alias

As an example:

module Dummy; end
# String.prepend(Dummy)
class String
  alias_method(:old_plus, :+)
  def + other
    puts 'blah blah'
    old_plus(other)
  end
end

'a' + 'b'

> blah blah
module Dummy; end
String.prepend(Dummy)
class String
  alias_method(:old_plus, :+)
  def + other
    puts 'blah blah'
    old_plus(other)
  end
end

'a' + 'b'

>

Prepending after an alias does not affect the previous alias

Updated by joshuadreed (Josh Reed) 3 months ago

module Dummy; end
class Foo
  def ten
    10
  end
end

Foo.prepend(Dummy)
class Foo
  alias_method(:old_ten, :ten)
  def ten
    puts 'blah blah'
    old_ten
  end
end

Foo.new.ten

> blah blah

Okay, doesn't seem to affect every class.

I do suspect it may be to do with refinement, so I will check into that next.

Updated by joshuadreed (Josh Reed) 3 months ago

So, I set out to test my hypothesis regarding refinements. I've never used refinements prior to this, so it's still possible there's an interaction somewhere, but at least not in the way I was thinking.

module Dummy; end

class Foo
  def ten
    10
  end
end

module Bar
  refine Foo do
    def ten
      11
    end
  end
end

class Baz
  using Bar
  def check
    Foo.new.ten
  end
end

Baz.prepend(Dummy)

class Baz
  alias_method(:old_check, :check)
  def check
    puts 'blah blah'
    old_check
  end
end

p Baz.new.check

>blah blah
>11

Updated by joshuadreed (Josh Reed) 3 months ago

One more detail.

module Dummy; end
class String
  alias_method(:old_plus, :+)
  def + other
    puts 'blah blah'
    old_plus(other)
  end
end


String.prepend(Dummy)

class String
  alias_method(:old_plus2, :+)
  def + other
    puts 'blah blah2'
    old_plus2(other)
  end
end

>blah blah2
>blah blah

If the method was aliased prior to prepending, the method is perfectly re-alias-able.

Updated by nobu (Nobuyoshi Nakada) 3 months ago

  • Subject changed from Prepend Breaks Ability to Alias to Prepend breaks ability to override optimized methods

Alias is not concerned.

# bug-17725.rb
String.prepend(Module.new) unless ARGV.empty?
class String
  def + other
    'blah blah'
  end
end

p 'a' + 'b'
$ ruby bug-17725.rb 
"blah blah"

$ ruby bug-17725.rb 1
"ab"

Seems the redefinition flag is not set by prepend.

Updated by nobu (Nobuyoshi Nakada) 3 months ago

nobu (Nobuyoshi Nakada) wrote in #note-4:

Seems the redefinition flag is not set by prepend.

This is not accurate, actual method owner class needs to be checked too.

Updated by nobu (Nobuyoshi Nakada) 3 months ago

Hmmmm?

$ ruby -v
ruby 3.1.0dev (2021-03-16T15:19:37Z master a47697aa44) [x86_64-darwin19]

$ ruby --enable=gems bug-17725.rb 
"blah blah"

$ ruby --enable=gems bug-17725.rb 1
"ab"

$ ruby --disable=gems bug-17725.rb
"blah blah"

$ ruby --disable=gems bug-17725.rb 1
"blah blah"

$ ruby --disable=gems -rrubygems bug-17725.rb
"blah blah"

$ ruby --disable=gems -rrubygems bug-17725.rb 1
"ab"

Updated by xtkoba (Tee KOBAYASHI) 3 months ago

FWIW,

# bug17725-test.rb
eval <<EOS
# encoding: ascii-8bit
MY_STRING = ''
EOS

MY_STRING + ''

String.prepend(Module.new)
class String
  def + other
    'blah blah'
  end
end

p 'a' + 'b'

gives

$ ruby --disable-gems bug17725-test.rb
"ab"
Actions #8

Updated by nobu (Nobuyoshi Nakada) 3 months ago

  • Backport changed from 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN to 2.5: DONTNEED, 2.6: DONTNEED, 2.7: DONTNEED, 3.0: REQUIRED

Updated by xtkoba (Tee KOBAYASHI) 3 months ago

Another example:

'' != nil

module Blah
  def == other
    'blah blah'
  end
end
String.prepend(Blah)

p 'a' == 'b' # => false

It seems that rb_vm_check_redefinition_opt_method is not working correctly in these cases. If the issue were only with String#== and String#+, a (dirty) workaround would be as follows:

--- a/vm.c
+++ b/vm.c
@@ -1854,6 +1854,12 @@ rb_vm_check_redefinition_opt_method(cons

        ruby_vm_redefined_flag[bop] |= flag;
    }
+   else if (me->def->body.iseq.iseqptr == (rb_iseq_t *)rb_str_equal) {
+       ruby_vm_redefined_flag[BOP_EQ] |= STRING_REDEFINED_OP_FLAG;
+   }
+   else if (me->def->body.iseq.iseqptr == (rb_iseq_t *)rb_str_plus) {
+       ruby_vm_redefined_flag[BOP_PLUS] |= STRING_REDEFINED_OP_FLAG;
+   }
     }
 }

Updated by jeremyevans0 (Jeremy Evans) 3 months ago

I've submitted a pull request to fix this: https://github.com/ruby/ruby/pull/4376

Actions

Also available in: Atom PDF