Bug #17822: Inconsistent visibility behavior with refinements

• Status: Closed
• Assignee: -
• Target version: -
• Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: DONE

[alanwu (Alan Wu)]

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 {

[ko1 (Koichi Sasada)]

Thank you the patch. It seems good. Could you commit with a test?

[alanwu (Alan Wu)]

• 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]

[nagachika (Tomoyuki Chikanaga)]

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

[nagachika (Tomoyuki Chikanaga)]

• 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.