Project

General

Profile

Actions

Bug #17822

closed

Inconsistent visibility behavior with refinements

Added by alanwu (Alan Wu) 9 months ago. Updated 8 months ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:103576]

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 {
Actions

Also available in: Atom PDF