Project

General

Profile

Actions

Bug #17822

closed

Inconsistent visibility behavior with refinements

Added by alanwu (Alan Wu) about 2 months ago. Updated 24 days 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 {

Updated by ko1 (Koichi Sasada) about 1 month ago

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

Actions #2

Updated by alanwu (Alan Wu) about 1 month 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]

Actions #3

Updated by nagachika (Tomoyuki Chikanaga) about 1 month 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) 24 days 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.

Actions

Also available in: Atom PDF