Bug #17822
closedInconsistent visibility behavior with refinements
Description
Running the following script, case 0 raises NoMethodError
for privacy violation,
while all other cases print :refined
. Shouldn't all three cases be equivalent?
class A; end
module M
refine(A) { def foo; :refined; end }
end
case ARGV.first.to_i
when 0
class A
private def foo; end
end
when 1
class A
prepend(Module.new)
private def foo; end
end
when 2
class A
private
def foo; end
end
end
using(M)
p(A.new.foo)
Some analysis: case 0 on master(cb78aae) behaves differently from version 3.0.1.
Applying the patch from https://github.com/ruby/ruby/pull/4357 recovers the behavior
found on 3.0.1
. That patch plus the following diff makes case 0 behave like case 1 and 2.
diff --git a/vm_method.c b/vm_method.c
index 0f25c514a8..82ac3a60b3 100644
--- a/vm_method.c
+++ b/vm_method.c
@@ -1399,11 +1399,16 @@ rb_export_method(VALUE klass, ID name, rb_method_visibility_t visi)
rb_vm_check_redefinition_opt_method(me, klass);
if (klass == defined_class || origin_class == defined_class) {
- METHOD_ENTRY_VISI_SET(me, visi);
-
- if (me->def->type == VM_METHOD_TYPE_REFINED && me->def->body.refined.orig_me) {
- METHOD_ENTRY_VISI_SET((rb_method_entry_t *)me->def->body.refined.orig_me, visi);
- }
+ if (me->def->type == VM_METHOD_TYPE_REFINED) {
+ // Refinement method entries should always be public because the refinement
+ // search is always performed.
+ if (me->def->body.refined.orig_me) {
+ METHOD_ENTRY_VISI_SET((rb_method_entry_t *)me->def->body.refined.orig_me, visi);
+ }
+ }
+ else {
+ METHOD_ENTRY_VISI_SET(me, visi);
+ }
rb_clear_method_cache(klass, name);
}
else {
Updated by ko1 (Koichi Sasada) over 3 years ago
Thank you the patch. It seems good. Could you commit with a test?
Updated by alanwu (Alan Wu) over 3 years ago
- Status changed from Open to Closed
Applied in changeset git|636d4f7eb9f3fcb088e1a44af4181c4aa36789b4.
Avoid setting the visibility of refinement method entries
Since refinement search is always performed, these entries should always
be public. The method entry that the refinement search returns decides
the visibility.
Fixes [Bug #17822]
Updated by nagachika (Tomoyuki Chikanaga) over 3 years ago
- Backport changed from 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN to 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: REQUIRED
Updated by nagachika (Tomoyuki Chikanaga) over 3 years ago
- Backport changed from 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: REQUIRED to 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: DONE
ruby_3_0 a21ec8d1ecc6e978ea6b18a27046c424e2849752 merged revision(s) 636d4f7eb9f3fcb088e1a44af4181c4aa36789b4.