Bug #7572

クラス定義においてスーパークラスとしてクラスでないものを指定してもエラーにならない事がある

Added by tadayoshi funaba over 2 years ago. Updated over 2 years ago.

[ruby-dev:46747]
Status:Closed
Priority:Normal
Assignee:Koichi Sasada
ruby -v:ruby 2.0.0dev (2012-12-16 trunk 38415) [i686-linux] Backport:

Description

クラス定義においてスーパークラスとしてクラスでないものを指定してもエラーにならない事がある。

$ ruby -v -e 'class Foo < nil; end'
ruby 2.0.0dev (2012-12-16 trunk 38415) [i686-linux]

$ ruby -v -e 'class Foo < false; end'
ruby 2.0.0dev (2012-12-16 trunk 38415) [i686-linux]

$ ruby -v -e 'class Foo < true; end'
ruby 2.0.0dev (2012-12-16 trunk 38415) [i686-linux]
-e:1:in `': wrong argument type true (expected Class) (TypeError)

defineclass_fix.diff Magnifier (1.88 KB) Shugo Maeda, 12/17/2012 11:00 PM

defineclass_fix_1220.diff Magnifier (5.96 KB) Shugo Maeda, 12/20/2012 02:27 PM

Associated revisions

Revision 38495
Added by Shugo Maeda over 2 years ago

  • vm_core.h (rb_vm_defineclass_type_t),
    compile.c (iseq_compile_each), insns.def (defineclass): change the
    meaning of the third operand of defineclass as follows:
    lower 3bits: the type of the defineclass
    0 = class, 1 = singleton class, 2 = module
    4th bit: a flag represents whether the defineclass is scoped
    0 = not scoped (e.g., class Foo)
    1 = scoped (e.g., class Bar::Baz)
    5th bit: a flag represents whether the superclass is specified
    0 = not specified (e.g., class Foo)
    1 = specified (e.g., class Bar < Foo)
    If the superclass is specified and is not a class, a TypeError
    should be raised. [Bug #7572]

  • test/ruby/test_class.rb: related test.

Revision 38495
Added by Shugo Maeda over 2 years ago

  • vm_core.h (rb_vm_defineclass_type_t),
    compile.c (iseq_compile_each), insns.def (defineclass): change the
    meaning of the third operand of defineclass as follows:
    lower 3bits: the type of the defineclass
    0 = class, 1 = singleton class, 2 = module
    4th bit: a flag represents whether the defineclass is scoped
    0 = not scoped (e.g., class Foo)
    1 = scoped (e.g., class Bar::Baz)
    5th bit: a flag represents whether the superclass is specified
    0 = not specified (e.g., class Foo)
    1 = specified (e.g., class Bar < Foo)
    If the superclass is specified and is not a class, a TypeError
    should be raised. [Bug #7572]

  • test/ruby/test_class.rb: related test.

History

#1 Updated by Shugo Maeda over 2 years ago

  • File defineclass_fix.diffMagnifier added
  • Status changed from Open to Assigned
  • Assignee set to Koichi Sasada

前田です。

tadf (tadayoshi funaba) wrote:

クラス定義においてスーパークラスとしてクラスでないものを指定してもエラーにならない事がある。

$ ruby -v -e 'class Foo < nil; end'
ruby 2.0.0dev (2012-12-16 trunk 38415) [i686-linux]

以下のように、コンパイルすると'class Foo; end'と'class Foo < nil; end'が同じ命令列に
なってしまうためのようです。

$ cat defineclass.rb
ary = RubyVM::InstructionSequence.compile(<@>==========
0000 trace 1 ( 1)
0002 putspecialobject 3
0004 putnil

0005 defineclass :Foo, class:Foo, 3
0009 pop

0010 trace 1 ( 2)
0012 putspecialobject 3
0014 putnil

0015 defineclass :Foo, class:Foo, 3
0019 leave

...

define_typeにスーパークラスを指定したかどうかを表すフラグを追加したパッチを添付します。
フラグの渡し方とかマジックナンバーでいいのかとか色々気になりますが、どうするのがよいでしょう? > ささださん

スーパークラスが指定されなかった場合はnilでなくQundefをputobjectしたらどうかとも
思ったのですが、r12621を見るとQundefはVMスタックに置かない方針なのですよね?

#2 Updated by Koichi Sasada over 2 years ago

方針はその方向で,少し整理するといいでしょうか.

スーパークラスが指定されなかった場合はnilでなくQundefをputobjectしたらどうかとも 思ったのですが、r12621を見るとQundefはVMスタックに置かない方針なのですよね?

はい,GC で mark 出来るものしか置けないようにしています.
(GC の時,ちょっとは速度的に楽になるかな,という配慮です)

#3 Updated by Shugo Maeda over 2 years ago

前田です。

ko1 (Koichi Sasada) wrote:

方針はその方向で,少し整理するといいでしょうか.

今のdefine_typeの値の付け方にとくに意味がないのであれば、scopedかどうか(class Foo::Barのような形か)も
フラグにしてしまった方がすっきりする気がします。

例えば、以下のようにしてはどうでしょうか。

下位3ビット -> 定義のタイプ (0 = クラス, 1 = 特異クラス, 2 = モジュール, 3以上は予約)
下から4ビット目 -> scopedなら1、そうでなければ0
下から5ビット目 -> スーパークラスが指定されていれば1、そうでなければ0

具体的には以下のような定義を考えています。

typedef enum {
VM_DEFINE_TYPE_CLASS = 0x00,
VM_DEFINE_TYPE_SINGLETON_CLASS = 0x01,
VM_DEFINE_TYPE_MODULE = 0x02,
/* 0x03..0x06 is reserved */
VM_DEFINE_TYPE_MASK = 0x07,
} rb_vm_define_type_t;

#define VM_DEFINE_FLAG_SCOPED 0x08
#define VM_DEFINE_FLAG_HAS_SUPERCLASS 0x10

その方針で作成したパッチを添付します。
i686-linux (Ubuntu 12.04)上でmake checkが通ることは確認しています。

#4 Updated by Koichi Sasada over 2 years ago

(2012/12/20 14:27), shugo (Shugo Maeda) wrote:

今のdefine_typeの値の付け方にとくに意味がないのであれば、scopedかどうか(class Foo::Barのような形か)も
フラグにしてしまった方がすっきりする気がします。

例えば、以下のようにしてはどうでしょうか。

下位3ビット -> 定義のタイプ (0 = クラス, 1 = 特異クラス, 2 = モジュール, 3以上は予約)
下から4ビット目 -> scopedなら1、そうでなければ0
下から5ビット目 -> スーパークラスが指定されていれば1、そうでなければ0

具体的には以下のような定義を考えています。

typedef enum {
VM_DEFINE_TYPE_CLASS = 0x00,
VM_DEFINE_TYPE_SINGLETON_CLASS = 0x01,
VM_DEFINE_TYPE_MODULE = 0x02,
/* 0x03..0x06 is reserved */
VM_DEFINE_TYPE_MASK = 0x07,
} rb_vm_define_type_t;

#define VM_DEFINE_FLAG_SCOPED 0x08
#define VM_DEFINE_FLAG_HAS_SUPERCLASS 0x10

その方針で作成したパッチを添付します。
i686-linux (Ubuntu 12.04)上でmake checkが通ることは確認しています。

 どうもありがとうございます.1点だけ,define_type を defineclass_type
にして頂ければ.

 コミット頂いてもいいでしょうか.

--
// SASADA Koichi at atdot dot net

#5 Updated by Shugo Maeda over 2 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r38495.
tadayoshi, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • vm_core.h (rb_vm_defineclass_type_t),
    compile.c (iseq_compile_each), insns.def (defineclass): change the
    meaning of the third operand of defineclass as follows:
    lower 3bits: the type of the defineclass
    0 = class, 1 = singleton class, 2 = module
    4th bit: a flag represents whether the defineclass is scoped
    0 = not scoped (e.g., class Foo)
    1 = scoped (e.g., class Bar::Baz)
    5th bit: a flag represents whether the superclass is specified
    0 = not specified (e.g., class Foo)
    1 = specified (e.g., class Bar < Foo)
    If the superclass is specified and is not a class, a TypeError
    should be raised. [Bug #7572]

  • test/ruby/test_class.rb: related test.

#6 Updated by Shugo Maeda over 2 years ago

前田です。

ko1 (Koichi Sasada) wrote:

 どうもありがとうございます.1点だけ,define_type を defineclass_type
にして頂ければ.

 コミット頂いてもいいでしょうか.

そのように修正してcommitしました。

Also available in: Atom PDF