Bug #10765
closedModule#remove_method remove refined method entry.
Description
Module#remove_method
should raise a NameError
if method is not defined in refined class, such as undef
.
But if method is defined in refined class, Module#remove_method
should keep refined method and remove original method.
I confirmed by following examples in ruby-trunk, 2.2.0, 2.1.5, 2.0.0-p598
class C
def foo
"C#foo"
end
end
module RefinementBug
refine C do
def foo
"RefinementBug#foo"
end
end
end
using RefinementBug
class C
remove_method :foo
end
puts C.new.foo
# expected:
# RefinementBug#foo
#
# actual:
# bug.rb:21:in `<main>': undefined method `foo' for #<C:0x007f9e5c087b48> (NoMethodError)
class C
end
module RefinementBug
refine C do
def foo
end
end
end
using RefinementBug
class C
remove_method :foo
end
# expected:
# bug2.rb:14:in `remove_method': method `foo' not defined in C (NameError)
# from bug2.rb:14:in `<class:C>'
# from bug2.rb:13:in `<main>'
#
# actual:
# # => nothing raised.
Files
Updated by hanachin (Seiei Miyagi) about 9 years ago
- File 0001-vm_method.c-fix-remove-refined-method.patch 0001-vm_method.c-fix-remove-refined-method.patch added
I attached a patch for this.
Updated by shugo (Shugo Maeda) about 9 years ago
- Status changed from Open to Assigned
- Assignee set to shugo (Shugo Maeda)
Updated by shugo (Shugo Maeda) about 9 years ago
- Status changed from Assigned to Closed
- % Done changed from 0 to 100
Applied in changeset r49480.
-
vm_method.c (remove_method): When remove refined
method, raise a NameError if the method is not
defined in refined class.But if the method is defined in refined class,
it should keep refined method and remove original
method.Patch by Seiei Higa. [ruby-core:67722] [Bug #10765]
Updated by shugo (Shugo Maeda) about 9 years ago
- Backport changed from 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN to 2.0.0: REQUIRED, 2.1: REQUIRED, 2.2: REQUIRED
Updated by vo.x (Vit Ondruch) about 9 years ago
- Status changed from Closed to Assigned
This breaks CentOS7, OpenSuse and Fedora at minimum:
http://rubyci.blob.core.windows.net/opensuse13/ruby-trunk/log/20150203T123003Z.log.html.gz
http://rubyci.blob.core.windows.net/fedora20/ruby-trunk/log/20150203T110002Z.log.html.gz
http://rubyci.blob.core.windows.net/centos7/ruby-trunk/log/20150203T110002Z.log.html.gz
/home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/.ext/common/json/common.rb:60:in `remove_method': method `to_json' not defined in Hash (NameError)
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/.ext/common/json/common.rb:60:in `block (3 levels) in generator='
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/.ext/common/json/common.rb:59:in `each'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/.ext/common/json/common.rb:59:in `block (2 levels) in generator='
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/.ext/common/json/common.rb:58:in `class_eval'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/.ext/common/json/common.rb:58:in `block in generator='
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/.ext/common/json/common.rb:55:in `each'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/.ext/common/json/common.rb:55:in `generator='
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/.ext/common/json/ext.rb:17:in `<module:Ext>'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/.ext/common/json/ext.rb:12:in `<module:JSON>'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/.ext/common/json/ext.rb:9:in `<top (required)>'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/lib/rubygems/core_ext/kernel_require.rb:54:in `require'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/lib/rubygems/core_ext/kernel_require.rb:54:in `require'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/.ext/common/json.rb:58:in `<module:JSON>'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/.ext/common/json.rb:54:in `<top (required)>'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/lib/rubygems/core_ext/kernel_require.rb:54:in `require'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/lib/rubygems/core_ext/kernel_require.rb:54:in `require'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/test/json/setup_variant.rb:10:in `<top (required)>'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/lib/rubygems/core_ext/kernel_require.rb:54:in `require'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/lib/rubygems/core_ext/kernel_require.rb:54:in `require'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/test/json/test_json.rb:5:in `<top (required)>'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/lib/rubygems/core_ext/kernel_require.rb:54:in `require'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/lib/rubygems/core_ext/kernel_require.rb:54:in `require'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/test/lib/test/unit.rb:826:in `block in non_options'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/test/lib/test/unit.rb:820:in `each'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/test/lib/test/unit.rb:820:in `non_options'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/test/lib/test/unit.rb:63:in `process_args'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/test/lib/test/unit.rb:101:in `process_args'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/test/lib/test/unit.rb:962:in `process_args'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/test/lib/test/unit.rb:967:in `run'
from /home/hsbt/chkbuild/tmp/build/20150203T110002Z/ruby/test/lib/test/unit.rb:974:in `run'
from ./test/runner.rb:40:in `<main>'
gmake: *** [yes-test-all] Error 1
exit 2
Updated by hanachin (Seiei Miyagi) about 9 years ago
If touch the me
after rb_unlink_method_entry
, it could cause error?
diff --git vm_method.c vm_method.c
index 8ad2b72..41a311c 100644
--- vm_method.c
+++ vm_method.c
@@ -775,11 +775,13 @@ remove_method(VALUE klass, ID mid)
key = (st_data_t)mid;
st_delete(RCLASS_M_TBL(klass), &key, &data);
+ int keep_refined_method_entry = me->def->type == VM_METHOD_TYPE_REFINED;
+
rb_vm_check_redefinition_opt_method(me, klass);
rb_clear_method_cache_by_class(klass);
rb_unlink_method_entry(me);
- if (me->def->type == VM_METHOD_TYPE_REFINED) {
+ if (keep_refined_method_entry) {
rb_add_refined_method_entry(klass, mid);
}
Updated by shugo (Shugo Maeda) about 9 years ago
Seiei Higa wrote:
If touch the
me
afterrb_unlink_method_entry
, it could cause error?
It's not the problem.
r49480 exposed the following potential problem:
class X
def foo
end
end
class Y < X
end
module Bar
refine Y do
def foo
end
end
end
p Y.instance_methods(false).include?(:foo) # false expected, but true is returned
Updated by shugo (Shugo Maeda) about 9 years ago
- Related to Bug #10826: Refinements make instance_methods(false) return methods of superclasses added
Updated by shugo (Shugo Maeda) about 9 years ago
The problem reported by Vit can be reproduced by the following command:
$ make test-all TESTS="test/ruby/test_refinement.rb test/json"
Updated by shugo (Shugo Maeda) about 9 years ago
- Status changed from Assigned to Closed
Fixed in r49493.
Updated by vo.x (Vit Ondruch) about 9 years ago
Testing with r49495 and it seems to be fixed. Thanks.
Updated by naruse (Yui NARUSE) about 9 years ago
- Backport changed from 2.0.0: REQUIRED, 2.1: REQUIRED, 2.2: REQUIRED to 2.0.0: REQUIRED, 2.1: REQUIRED, 2.2: DONE
ruby_2_2 r49647 merged revision(s) 49480,49493.
Updated by usa (Usaku NAKAMURA) about 9 years ago
- Backport changed from 2.0.0: REQUIRED, 2.1: REQUIRED, 2.2: DONE to 2.0.0: DONE, 2.1: REQUIRED, 2.2: DONE
ruby_2_0_0 r49738 merged revision(s) 49222,49480,49493.
Updated by nagachika (Tomoyuki Chikanaga) about 9 years ago
- Backport changed from 2.0.0: DONE, 2.1: REQUIRED, 2.2: DONE to 2.0.0: DONE, 2.1: DONE, 2.2: DONE
Backported into ruby_2_1
branch at r49992.