Project

General

Profile

Bug #11173

inter class/module alias causes "no superclass method"

Added by ko1 (Koichi Sasada) over 4 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-dev:48992]

Description

class C0
  def foo
    p :C0
  end
end

module M0
  def foo
    p :M0
    super
  end
end

module M1
  include M0
  alias orig_foo foo
end

class C1 < C0
  include M1
end

p C1.ancestors
C1.new.orig_foo

結果:

ruby 2.3.0dev (2015-05-21 trunk 50502) [i386-mswin32_110]
[C1, M1, M0, C0, Object, Kernel, BasicObject]
:M0
test.rb:12:in `foo': super: no superclass method `foo' for #<C1:0x109634c> (NoMethodError)
    from test.rb:26:in `<main>'

M0#foo の super である C0#foo が呼ばれてもいいと思うのですが、
この挙動はそういうもんでしたっけ。

挙動的には、M1 の alias で作られる method_entry が M0 の T_ICLASS (TI1) を指すけど、C1 からの継承関係で作られる M0 を指す T_ICLASS (TI2)と、TI1 が関係ないから、起こっているんだと思います。

なんか、class/module をまたいだ alias の挙動(と、その super での挙動)が色々怪しい感じです。

Associated revisions

Revision f1d4e8b3
Added by ko1 (Koichi Sasada) over 4 years ago

  • method.h: add VM_METHOD_TYPE_ALIAS rb_method_definition_t::type to fix [Bug #11173]. Now, inter class/method alias creates new method entry VM_METHOD_TYPE_ALIAS, which has an original method entry.
  • vm_insnhelper.c (find_defiend_class_by_owner): added. Search corresponding defined_class from owner class/module.
  • vm_method.c (rb_method_entry_get_without_cache): return me->klass directly for defined_class. Now, no need to check me->klass any more.
  • vm_method.c (method_entry_set0): separated from method_entry_set().
  • vm_method.c (rb_alias): make method entry has VM_METHOD_TYPE_ALIAS.
  • vm_method.c (release_method_definition): support VM_METHOD_TYPE_ALIAS.
  • vm_method.c (rb_hash_method_definition): ditto.
  • vm_method.c (rb_method_definition_eq): ditto.
  • vm_method.c (release_method_definition): ditto.
  • vm_insnhelper.c (vm_call_method): ditto.
  • vm_insnhelper.c (vm_method_cfunc_entry): ditto.
  • vm_eval.c (vm_call0_body): ditto.
  • gc.c (mark_method_entry): ditto.
  • proc.c (method_def_iseq): ditto.
  • proc.c (method_cref): ditto.
  • proc.c (rb_method_entry_min_max_arity): ditto.
  • test/ruby/test_alias.rb: add tests.
  • test/ruby/test_module.rb: fix a test to catch up current behavior.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@50691 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 50691
Added by ko1 (Koichi Sasada) over 4 years ago

  • method.h: add VM_METHOD_TYPE_ALIAS rb_method_definition_t::type to fix [Bug #11173]. Now, inter class/method alias creates new method entry VM_METHOD_TYPE_ALIAS, which has an original method entry.
  • vm_insnhelper.c (find_defiend_class_by_owner): added. Search corresponding defined_class from owner class/module.
  • vm_method.c (rb_method_entry_get_without_cache): return me->klass directly for defined_class. Now, no need to check me->klass any more.
  • vm_method.c (method_entry_set0): separated from method_entry_set().
  • vm_method.c (rb_alias): make method entry has VM_METHOD_TYPE_ALIAS.
  • vm_method.c (release_method_definition): support VM_METHOD_TYPE_ALIAS.
  • vm_method.c (rb_hash_method_definition): ditto.
  • vm_method.c (rb_method_definition_eq): ditto.
  • vm_method.c (release_method_definition): ditto.
  • vm_insnhelper.c (vm_call_method): ditto.
  • vm_insnhelper.c (vm_method_cfunc_entry): ditto.
  • vm_eval.c (vm_call0_body): ditto.
  • gc.c (mark_method_entry): ditto.
  • proc.c (method_def_iseq): ditto.
  • proc.c (method_cref): ditto.
  • proc.c (rb_method_entry_min_max_arity): ditto.
  • test/ruby/test_alias.rb: add tests.
  • test/ruby/test_module.rb: fix a test to catch up current behavior.

Revision 50691
Added by ko1 (Koichi Sasada) over 4 years ago

  • method.h: add VM_METHOD_TYPE_ALIAS rb_method_definition_t::type to fix [Bug #11173]. Now, inter class/method alias creates new method entry VM_METHOD_TYPE_ALIAS, which has an original method entry.
  • vm_insnhelper.c (find_defiend_class_by_owner): added. Search corresponding defined_class from owner class/module.
  • vm_method.c (rb_method_entry_get_without_cache): return me->klass directly for defined_class. Now, no need to check me->klass any more.
  • vm_method.c (method_entry_set0): separated from method_entry_set().
  • vm_method.c (rb_alias): make method entry has VM_METHOD_TYPE_ALIAS.
  • vm_method.c (release_method_definition): support VM_METHOD_TYPE_ALIAS.
  • vm_method.c (rb_hash_method_definition): ditto.
  • vm_method.c (rb_method_definition_eq): ditto.
  • vm_method.c (release_method_definition): ditto.
  • vm_insnhelper.c (vm_call_method): ditto.
  • vm_insnhelper.c (vm_method_cfunc_entry): ditto.
  • vm_eval.c (vm_call0_body): ditto.
  • gc.c (mark_method_entry): ditto.
  • proc.c (method_def_iseq): ditto.
  • proc.c (method_cref): ditto.
  • proc.c (rb_method_entry_min_max_arity): ditto.
  • test/ruby/test_alias.rb: add tests.
  • test/ruby/test_module.rb: fix a test to catch up current behavior.

Revision 50691
Added by ko1 (Koichi Sasada) over 4 years ago

  • method.h: add VM_METHOD_TYPE_ALIAS rb_method_definition_t::type to fix [Bug #11173]. Now, inter class/method alias creates new method entry VM_METHOD_TYPE_ALIAS, which has an original method entry.
  • vm_insnhelper.c (find_defiend_class_by_owner): added. Search corresponding defined_class from owner class/module.
  • vm_method.c (rb_method_entry_get_without_cache): return me->klass directly for defined_class. Now, no need to check me->klass any more.
  • vm_method.c (method_entry_set0): separated from method_entry_set().
  • vm_method.c (rb_alias): make method entry has VM_METHOD_TYPE_ALIAS.
  • vm_method.c (release_method_definition): support VM_METHOD_TYPE_ALIAS.
  • vm_method.c (rb_hash_method_definition): ditto.
  • vm_method.c (rb_method_definition_eq): ditto.
  • vm_method.c (release_method_definition): ditto.
  • vm_insnhelper.c (vm_call_method): ditto.
  • vm_insnhelper.c (vm_method_cfunc_entry): ditto.
  • vm_eval.c (vm_call0_body): ditto.
  • gc.c (mark_method_entry): ditto.
  • proc.c (method_def_iseq): ditto.
  • proc.c (method_cref): ditto.
  • proc.c (rb_method_entry_min_max_arity): ditto.
  • test/ruby/test_alias.rb: add tests.
  • test/ruby/test_module.rb: fix a test to catch up current behavior.

Revision 50691
Added by ko1 (Koichi Sasada) over 4 years ago

  • method.h: add VM_METHOD_TYPE_ALIAS rb_method_definition_t::type to fix [Bug #11173]. Now, inter class/method alias creates new method entry VM_METHOD_TYPE_ALIAS, which has an original method entry.
  • vm_insnhelper.c (find_defiend_class_by_owner): added. Search corresponding defined_class from owner class/module.
  • vm_method.c (rb_method_entry_get_without_cache): return me->klass directly for defined_class. Now, no need to check me->klass any more.
  • vm_method.c (method_entry_set0): separated from method_entry_set().
  • vm_method.c (rb_alias): make method entry has VM_METHOD_TYPE_ALIAS.
  • vm_method.c (release_method_definition): support VM_METHOD_TYPE_ALIAS.
  • vm_method.c (rb_hash_method_definition): ditto.
  • vm_method.c (rb_method_definition_eq): ditto.
  • vm_method.c (release_method_definition): ditto.
  • vm_insnhelper.c (vm_call_method): ditto.
  • vm_insnhelper.c (vm_method_cfunc_entry): ditto.
  • vm_eval.c (vm_call0_body): ditto.
  • gc.c (mark_method_entry): ditto.
  • proc.c (method_def_iseq): ditto.
  • proc.c (method_cref): ditto.
  • proc.c (rb_method_entry_min_max_arity): ditto.
  • test/ruby/test_alias.rb: add tests.
  • test/ruby/test_module.rb: fix a test to catch up current behavior.

Revision 50691
Added by ko1 (Koichi Sasada) over 4 years ago

  • method.h: add VM_METHOD_TYPE_ALIAS rb_method_definition_t::type to fix [Bug #11173]. Now, inter class/method alias creates new method entry VM_METHOD_TYPE_ALIAS, which has an original method entry.
  • vm_insnhelper.c (find_defiend_class_by_owner): added. Search corresponding defined_class from owner class/module.
  • vm_method.c (rb_method_entry_get_without_cache): return me->klass directly for defined_class. Now, no need to check me->klass any more.
  • vm_method.c (method_entry_set0): separated from method_entry_set().
  • vm_method.c (rb_alias): make method entry has VM_METHOD_TYPE_ALIAS.
  • vm_method.c (release_method_definition): support VM_METHOD_TYPE_ALIAS.
  • vm_method.c (rb_hash_method_definition): ditto.
  • vm_method.c (rb_method_definition_eq): ditto.
  • vm_method.c (release_method_definition): ditto.
  • vm_insnhelper.c (vm_call_method): ditto.
  • vm_insnhelper.c (vm_method_cfunc_entry): ditto.
  • vm_eval.c (vm_call0_body): ditto.
  • gc.c (mark_method_entry): ditto.
  • proc.c (method_def_iseq): ditto.
  • proc.c (method_cref): ditto.
  • proc.c (rb_method_entry_min_max_arity): ditto.
  • test/ruby/test_alias.rb: add tests.
  • test/ruby/test_module.rb: fix a test to catch up current behavior.

History

Updated by ko1 (Koichi Sasada) over 4 years ago

似たような話なんですが、

module M0
  private
  def foo
    p M0
  end
end

class C0
  def foo
    p C0
  end
end

class C1 < C0
  include M0
  public :foo
end

module M0
  remove_method :foo
end

C1.new.foo

これだと、C1#foo は METHOD_TYPE_ZSUPER として定義されており、M0#foo が無くなったので、C0#foo を探し出し、無事に C0#foo が実行されます。

しかし、

module M0
  private
  def foo
    p M0
  end
end

class C0
  def foo
    p C0
  end
end

module M1
  include M0
  public :foo
end

module M0
  remove_method :foo
end

class C1 < C0
  include M1
end

C1.new.foo
#=> test.rb:27:in `<main>': undefined method `foo' for #<C1:0x287bf0c> (NoMethodError)

と、(多分)ちゃんと動きません。M1#foo が ZSUPER なのだけれど、M1 の T_ICLASS より上を探そうとして、T_ICLASS(M0) を探して、見つからない → T_ICLASS(M0) の super は無いので、探索終了→メソッド見つからず、となります。

Updated by matz (Yukihiro Matsumoto) over 4 years ago

確かにおかしいですね。「似たような話」も含めて考えると混乱するので、いったん置いておくと、もとのプログラムはfooのスーパーをM0の親から探し始めて、C0#fooを見つけて欲しいような気もしますね。ただ、これは1.8でも同じ挙動をするので、なにか難しい問題があるのかもしれません。

mrubyで確認したら、あっちはaliasとsuperの関係が全然実装できてなかった。

Matz.

Updated by ko1 (Koichi Sasada) over 4 years ago

「この問題は trivial なので、2.3 では直すけど、2.2 以前は直さない」、ってのは許されるものでしょうか。

というのも、今メソッドディスパッチの部分を大きく書き直しており、別々のパッチを書かないといけなさそうなんですよね。

Updated by ko1 (Koichi Sasada) over 4 years ago

とりあえず、2.3dev 用のパッチを書いてみました。

クラス/モジュールをまたぐ alias が行なわれたときには、
メソッドエントリ ALIAS 型のメソッドを登録する、という実装にしています。
ALIAS 型は、owner class/module を把握しておき(me->def->body.alias.original_me->klass)、
メソッドディスパッチ時に継承空間から辿って owner を探し、それを defined_class として処理を続けます。

多分、これでいいと思うんだけど(test-all はほぼ通った)、ちょっと影響が読めず。

TestModule#test_inspect_segfault [/home/ko1/src/ruby/trunk/test/ruby/test_module.rb:2046]:
[ruby-core:65214] [Bug #10282].
<"#<Method: A(Object)#inspect(shallow_inspect)>"> expected but was
<"#<Method: A(ShallowInspect)#inspect(shallow_inspect)>">.

なのですが、これは元々のテストが悪い気がします。

あと、[Bug #11189] が問題で治らないのが 1 つ。
(チケット番号が間違えていたので修正)

Index: gc.c
===================================================================
--- gc.c    (revision 50652)
+++ gc.c    (working copy)
@@ -3959,6 +3959,9 @@
        goto again;
    }
    break;
+      case VM_METHOD_TYPE_ALIAS:
+   mark_method_entry(objspace, def->body.alias.original_me);
+   return;
       default:
    break; /* ignore */
     }
Index: method.h
===================================================================
--- method.h    (revision 50652)
+++ method.h    (working copy)
@@ -51,6 +51,7 @@
     VM_METHOD_TYPE_IVAR,
     VM_METHOD_TYPE_BMETHOD,
     VM_METHOD_TYPE_ZSUPER,
+    VM_METHOD_TYPE_ALIAS,
     VM_METHOD_TYPE_UNDEF,
     VM_METHOD_TYPE_NOTIMPLEMENTED,
     VM_METHOD_TYPE_OPTIMIZED, /* Kernel#send, Proc#call, etc */
@@ -71,6 +72,10 @@
     const VALUE location;
 } rb_method_attr_t;

+typedef struct rb_method_alias_struct {
+    const struct rb_method_entry_struct *original_me; /* original_me->klass is original owner */
+} rb_method_alias_t;
+
 typedef struct rb_iseq_struct rb_iseq_t;

 typedef struct rb_method_definition_struct {
@@ -85,6 +90,7 @@
    } iseq_body;
    rb_method_cfunc_t cfunc;
    rb_method_attr_t attr;
+   rb_method_alias_t alias;
    const VALUE proc;                 /* should be mark */
    enum method_optimized_type {
        OPTIMIZED_METHOD_TYPE_SEND,
Index: proc.c
===================================================================
--- proc.c  (revision 50652)
+++ proc.c  (working copy)
@@ -2066,6 +2066,8 @@
    return *max = 1;
       case VM_METHOD_TYPE_IVAR:
    return *max = 0;
+      case VM_METHOD_TYPE_ALIAS:
+   return rb_method_entry_min_max_arity(def->body.alias.original_me, max);
       case VM_METHOD_TYPE_BMETHOD:
    return rb_proc_min_max_arity(def->body.proc, max);
       case VM_METHOD_TYPE_ISEQ: {
@@ -2203,11 +2205,15 @@
 static const rb_iseq_t *
 method_def_iseq(const rb_method_definition_t *def)
 {
+  again:
     switch (def->type) {
       case VM_METHOD_TYPE_BMETHOD:
    return get_proc_iseq(def->body.proc, 0);
       case VM_METHOD_TYPE_ISEQ:
    return def->body.iseq_body.iseq;
+      case VM_METHOD_TYPE_ALIAS:
+   def =def->body.alias.original_me->def;
+   goto again;
       default:
    return NULL;
     }
@@ -2224,9 +2230,13 @@
 {
     const rb_method_definition_t *def = method_def(method);

+  again:
     switch (def->type) {
       case VM_METHOD_TYPE_ISEQ:
    return def->body.iseq_body.cref;
+      case VM_METHOD_TYPE_ALIAS:
+   def = def->body.alias.original_me->def;
+   goto again;
       default:
    return NULL;
     }
Index: vm_eval.c
===================================================================
--- vm_eval.c   (revision 50652)
+++ vm_eval.c   (working copy)
@@ -217,6 +217,12 @@
        if (!ci->me->def) return Qnil;
        goto again;
    }
+      case VM_METHOD_TYPE_ALIAS:
+   {
+       ci->me = ci->me->def->body.alias.original_me;
+       ci->defined_class = find_defiend_class_by_owner(ci->klass /* current class */, ci->me->klass /* owner */);
+       goto again;
+   }
       case VM_METHOD_TYPE_MISSING:
    {
        VALUE new_args = rb_ary_new4(ci->argc, argv);
Index: vm_insnhelper.c
===================================================================
--- vm_insnhelper.c (revision 50652)
+++ vm_insnhelper.c (working copy)
@@ -1382,6 +1382,7 @@
    METHOD_BUG(OPTIMIZED);
    METHOD_BUG(MISSING);
    METHOD_BUG(REFINED);
+   METHOD_BUG(ALIAS);
 # undef METHOD_BUG
       default:
    rb_bug("wrong method type: %d", me->def->type);
@@ -1700,6 +1701,22 @@
     return cfp;
 }

+static VALUE
+find_defiend_class_by_owner(VALUE current_class, VALUE target_owner)
+{
+    VALUE klass = current_class;
+
+    while (RTEST(klass)) {
+   VALUE owner = RB_TYPE_P(klass, T_ICLASS) ? RBASIC_CLASS(klass) : klass;
+   if (owner == target_owner) {
+       return klass;
+   }
+   klass = RCLASS_SUPER(klass);
+    }
+
+    return current_class;
+}
+
 static
 #ifdef _MSC_VER
 __forceinline
@@ -1771,6 +1788,11 @@
            goto start_method_dispatch;
        }
          }
+         case VM_METHOD_TYPE_ALIAS: {
+       ci->me = ci->me->def->body.alias.original_me;
+       ci->defined_class = find_defiend_class_by_owner(ci->defined_class, ci->me->klass /* owner */);
+       goto normal_method_dispatch;
+         }
          case VM_METHOD_TYPE_OPTIMIZED:{
        switch (ci->me->def->body.optimize_type) {
          case OPTIMIZED_METHOD_TYPE_SEND:
Index: vm_method.c
===================================================================
--- vm_method.c (revision 50652)
+++ vm_method.c (working copy)
@@ -537,13 +537,20 @@
 }

 static rb_method_entry_t *
+method_entry_set0(VALUE klass, ID mid, rb_method_type_t type,
+         rb_method_definition_t *def, rb_method_flag_t noex, VALUE defined_class)
+{
+    rb_method_entry_t *newme = rb_method_entry_make(klass, mid, type, def, noex, defined_class);
+    method_added(klass, mid);
+    return newme;
+}
+
+static rb_method_entry_t *
 method_entry_set(VALUE klass, ID mid, const rb_method_entry_t *me,
         rb_method_flag_t noex, VALUE defined_class)
 {
     rb_method_type_t type = me->def ? me->def->type : VM_METHOD_TYPE_UNDEF;
-    rb_method_entry_t *newme = rb_method_entry_make(klass, mid, type, me->def, noex, defined_class);
-    method_added(klass, mid);
-    return newme;
+    return method_entry_set0(klass, mid, type, me->def, noex, defined_class);
 }

 rb_method_entry_t *
@@ -613,16 +620,6 @@
     VALUE defined_class;
     rb_method_entry_t *me = search_method(klass, id, &defined_class);

-    if (me && me->klass) {
-   switch (BUILTIN_TYPE(me->klass)) {
-     case T_CLASS:
-       if (RBASIC(klass)->flags & FL_SINGLETON) break;
-       /* fall through */
-     case T_ICLASS:
-       defined_class = me->klass;
-   }
-    }
-
     if (ruby_running) {
    if (OPT_GLOBAL_METHOD_CACHE) {
        struct cache_entry *ent;
@@ -1321,14 +1318,28 @@
    flag = orig_me->flag;
    goto again;
     }
+
+    if (flag == NOEX_UNDEF) flag = orig_me->flag;
+
     if (RB_TYPE_P(defined_class, T_ICLASS)) {
-   VALUE real_class = RBASIC_CLASS(defined_class);
-   if (real_class && RCLASS_ORIGIN(real_class) == defined_class)
-       defined_class = real_class;
+   VALUE real_owner = RBASIC_CLASS(defined_class);
+   rb_method_definition_t *def;
+   defined_class = real_owner;
+
+   /* make alias def */
+   def = ALLOC(rb_method_definition_t);
+   def->type = VM_METHOD_TYPE_ALIAS;
+   def->original_id = orig_me->called_id;
+   def->alias_count = 0;
+   def->body.alias.original_me = orig_me;
+   if (orig_me->def) orig_me->def->alias_count++;
+
+   /* make copy */
+   method_entry_set0(target_klass, name, VM_METHOD_TYPE_ALIAS, def, flag, defined_class);
     }
-
-    if (flag == NOEX_UNDEF) flag = orig_me->flag;
-    method_entry_set(target_klass, name, orig_me, flag, defined_class);
+    else {
+   method_entry_set(target_klass, name, orig_me, flag, defined_class);
+    }
 }

 /*
#5

Updated by ko1 (Koichi Sasada) over 4 years ago

  • Status changed from Open to Closed

Applied in changeset r50691.


  • method.h: add VM_METHOD_TYPE_ALIAS rb_method_definition_t::type to fix [Bug #11173]. Now, inter class/method alias creates new method entry VM_METHOD_TYPE_ALIAS, which has an original method entry.
  • vm_insnhelper.c (find_defiend_class_by_owner): added. Search corresponding defined_class from owner class/module.
  • vm_method.c (rb_method_entry_get_without_cache): return me->klass directly for defined_class. Now, no need to check me->klass any more.
  • vm_method.c (method_entry_set0): separated from method_entry_set().
  • vm_method.c (rb_alias): make method entry has VM_METHOD_TYPE_ALIAS.
  • vm_method.c (release_method_definition): support VM_METHOD_TYPE_ALIAS.
  • vm_method.c (rb_hash_method_definition): ditto.
  • vm_method.c (rb_method_definition_eq): ditto.
  • vm_method.c (release_method_definition): ditto.
  • vm_insnhelper.c (vm_call_method): ditto.
  • vm_insnhelper.c (vm_method_cfunc_entry): ditto.
  • vm_eval.c (vm_call0_body): ditto.
  • gc.c (mark_method_entry): ditto.
  • proc.c (method_def_iseq): ditto.
  • proc.c (method_cref): ditto.
  • proc.c (rb_method_entry_min_max_arity): ditto.
  • test/ruby/test_alias.rb: add tests.
  • test/ruby/test_module.rb: fix a test to catch up current behavior.

Updated by usa (Usaku NAKAMURA) over 4 years ago

  • Backport changed from 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN to 2.0.0: WONTFIX, 2.1: REQUIRED, 2.2: REQUIRED

いったん2.1と2.2でREQUIREDとしてますが、必ずしも直さないといけないと思っているわけではありません。
(問題自体はあるよ、というマーク付け)

Also available in: Atom PDF