Bug #17725
openPrepend breaks ability to override optimized methods
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) 29 days 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) 29 days 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) 29 days 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) 29 days 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) 29 days 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) 29 days 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) 28 days 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"
Updated by nobu (Nobuyoshi Nakada) 28 days 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) 26 days 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) 5 days ago
I've submitted a pull request to fix this: https://github.com/ruby/ruby/pull/4376