Project

General

Profile

Actions

Feature #6936

closed

Forbid singleton class and instance variabls for float

Added by naruse (Yui NARUSE) over 10 years ago. Updated over 10 years ago.

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

Description

[Feature #6763] などで議論されていた flonum が r36798 でが入ったわけですが、

  1. Float のオブジェクトID の仕様が変更
  2. flonum な float に特異メソッドが追加不可
  3. flonum な float に特異クラスが作成不可
  4. flonum な float は同じ値同士でインスタンス変数が共有される

といった非互換が存在します。
もっとも、1. は通常意識するはずのないところですし、2. は元から禁止されています。
気になるのは 3. と 4. で、これは 1.9.3 と挙動が異なるだけでなく、
32bit 環境での 2.0 や、64bit環境の flonum でない float オブジェクトとも挙動が異なります。

実際問題として実害はないような気もしますが、このような違いが極めて実装上の問題で、
Ruby 上から見えないところに存在するのは気持ち悪く感じます。

よって、以下のようにするとよいのではないでしょうか。

  • flonum でない float でも特異クラスの作成を禁止
  • float へのインスタンス変数作成を禁止

後者の具体的手法はいくつかあると思いますが、即値は最初から frozen にしておくとかもありかなと思っています。

話を発散させると、この話は true, false, nil, Fixnum, Symbol のような即値から、
Bignum や Time のような immutable っぽいオブジェクトにも当てはまる気がしています。


Related issues 3 (0 open3 closed)

Related to Backport187 - Backport #4578: Fixnum.freeze not frozen?ClosedActions
Related to Ruby master - Feature #6763: Introduce Flonum technique to speedup floating computation on th 64bit environmentClosedko1 (Koichi Sasada)07/21/2012Actions
Related to Ruby master - Feature #3222: Can bignums have singleton class & methods?Closedko1 (Koichi Sasada)04/30/2010Actions

Updated by ko1 (Koichi Sasada) over 10 years ago

(2012/08/27 8:12), naruse (Yui NARUSE) wrote:

後者の具体的手法はいくつかあると思いますが、即値は最初から frozen にしておくとかもありかなと思っています。

 Numeric は freeze しちゃう,というのだとやり過ぎでしょうか.

--
// SASADA Koichi at atdot dot net

Updated by kosaki (Motohiro KOSAKI) over 10 years ago

後者の具体的手法はいくつかあると思いますが、即値は最初から frozen にしておくとかもありかなと思っています。

 Numeric は freeze しちゃう,というのだとやり過ぎでしょうか.

ぼくはありだとおもう。数という概念は世界がはじまるより前からあったのだ、ムハハハハハという世界ですな

Updated by naruse (Yui NARUSE) over 10 years ago

(2012/08/27 17:46), KOSAKI Motohiro wrote:

後者の具体的手法はいくつかあると思いますが、即値は最初から frozen にしておくとかもありかなと思っています。

 Numeric は freeze しちゃう,というのだとやり過ぎでしょうか.

ぼくはありだとおもう。数という概念は世界がはじまるより前からあったのだ、ムハハハハハという世界ですな

そうですね、そうすると統一感が出るんじゃないかなぁと思っています。

--
NARUSE, Yui

Updated by usa (Usaku NAKAMURA) over 10 years ago

こんにちは、なかむら(う)です。

In message "[ruby-dev:46082] Re: [ruby-trunk - Feature #6936][Assigned] Forbid singleton class and instance variabls for float"
on Aug.27,2012 15:22:07, wrote:

 Numeric は freeze しちゃう,というのだとやり過ぎでしょうか.

+1

それでは。

U.Nakamura

Updated by mrkn (Kenta Murata) over 10 years ago

むらたです。

2012/8/28 U.Nakamura :

こんにちは、なかむら(う)です。

In message "[ruby-dev:46082] Re: [ruby-trunk - Feature #6936][Assigned] Forbid singleton class and instance variabls for float"
on Aug.27,2012 15:22:07, wrote:

  Numeric は freeze しちゃう,というのだとやり過ぎでしょうか.

+1

わたしも +1。

--
Kenta Murata
OpenPGP FP = 1D69 ADDE 081C 9CC2 2E54 98C1 CEFE 8AFB 6081 B062

本を書きました!!
『Ruby 逆引きレシピ』 http://www.amazon.co.jp/dp/4798119881/mrkn-22

E-mail:
twitter: http://twitter.com/mrkn/
blog: http://d.hatena.ne.jp/mrkn/

Updated by Anonymous over 10 years ago

稲葉と申します。
部外者(Perl屋)からの茶々です。すみません。

(2012/08/28 22:04), Kenta Murata wrote:

むらたです。

2012/8/28 U.Nakamura :

こんにちは、なかむら(う)です。

In message "[ruby-dev:46082] Re: [ruby-trunk - Feature #6936][Assigned] Forbid singleton class and instance variabls for float"
on Aug.27,2012 15:22:07, wrote:

  Numeric は freeze しちゃう,というのだとやり過ぎでしょうか.

+1

わたしも +1。

こーゆうのが++でないのがRubyistか、って思いました。
(ひょっとしてRuby界では常識? +=1じゃないんですね。)

すみません。

 稲葉 浩人

Updated by kosaki (Motohiro KOSAKI) over 10 years ago

+1

わたしも +1。

こーゆうのが++でないのがRubyistか、って思いました。
(ひょっとしてRuby界では常識? +=1じゃないんですね。)

たぶんそういう言語美意識的な深い意味はなくてたんにruby-coreで +1 を使う人が
多いのでそれに影響されてるだけだと思いますよ

Updated by ko1 (Koichi Sasada) over 10 years ago

(2012/08/27 22:35), NARUSE, Yui wrote:

(2012/08/27 17:46), KOSAKI Motohiro wrote:

後者の具体的手法はいくつかあると思いますが、即値は最初から frozen にしておくとかもありかなと思っています。

 Numeric は freeze しちゃう,というのだとやり過ぎでしょうか.

ぼくはありだとおもう。数という概念は世界がはじまるより前からあったのだ、ムハハハハハという世界ですな

そうですね、そうすると統一感が出るんじゃないかなぁと思っています。

 とりあえず影響が少なそうな,Float だけ frozen にしちゃうのはどうでしょ
うか.

--
// SASADA Koichi at atdot dot net

Updated by naruse (Yui NARUSE) over 10 years ago

ko1 (Koichi Sasada) wrote:

(2012/08/27 22:35), NARUSE, Yui wrote:

(2012/08/27 17:46), KOSAKI Motohiro wrote:

後者の具体的手法はいくつかあると思いますが、即値は最初から frozen にしておくとかもありかなと思っています。

 Numeric は freeze しちゃう,というのだとやり過ぎでしょうか.

ぼくはありだとおもう。数という概念は世界がはじまるより前からあったのだ、ムハハハハハという世界ですな

そうですね、そうすると統一感が出るんじゃないかなぁと思っています。

 とりあえず影響が少なそうな,Float だけ frozen にしちゃうのはどうでしょ
うか.

以下のような感じですかね

diff --git a/class.c b/class.c
index 1d871fb..1df38e4 100644
--- a/class.c
+++ b/class.c
@@ -1324,6 +1324,10 @@ singleton_class_of(VALUE obj)
rb_bug("unknown immediate %p", (void *)obj);
return klass;
}

  • else {

  •   if (BUILTIN_TYPE(obj) == T_FLOAT)
    
  •       rb_raise(rb_eTypeError, "can't define singleton");
    
  • }

    if (FL_TEST(RBASIC(obj)->klass, FL_SINGLETON) &&
    rb_ivar_get(RBASIC(obj)->klass, id_attached) == obj) {
    diff --git a/include/ruby/ruby.h b/include/ruby/ruby.h
    index a674de8..53de6a8 100644
    --- a/include/ruby/ruby.h
    +++ b/include/ruby/ruby.h
    @@ -1129,7 +1129,7 @@ struct RBignum {
    (FL_TAINT | FL_UNTRUSTED);
    } while (0)

-#define OBJ_FROZEN(x) (!!FL_TEST((x), FL_FREEZE))
+#define OBJ_FROZEN(x) (!!(FL_ABLE(x)?(RBASIC(x)->flags&(FL_FREEZE)):FLONUM_P(x)))
#define OBJ_FREEZE(x) FL_SET((x), FL_FREEZE)

#if SIZEOF_INT < SIZEOF_LONG
diff --git a/numeric.c b/numeric.c
index 58ac7ad..6a72fba 100644
--- a/numeric.c
+++ b/numeric.c
@@ -621,6 +621,7 @@ rb_float_new_in_heap(double d)
OBJSETUP(flt, rb_cFloat, T_FLOAT);

 flt->float_value = d;
  • OBJ_FREEZE(flt);
    return (VALUE)flt;
    }

Updated by matz (Yukihiro Matsumoto) over 10 years ago

  • Assignee changed from matz (Yukihiro Matsumoto) to ko1 (Koichi Sasada)

OK, I'd like to see if everything goes well. Merge it.

Matz.

Updated by ko1 (Koichi Sasada) over 10 years ago

(2012/10/27 8:38), matz (Yukihiro Matsumoto) wrote:

Assignee changed from matz (Yukihiro Matsumoto) to ko1 (Koichi Sasada)

OK, I'd like to see if everything goes well. Merge it.

Roger, boss.

--
// SASADA Koichi at atdot dot net

Updated by ko1 (Koichi Sasada) over 10 years ago

(2012/10/27 9:05), SASADA Koichi wrote:

(2012/10/27 8:38), matz (Yukihiro Matsumoto) wrote:

Assignee changed from matz (Yukihiro Matsumoto) to ko1 (Koichi Sasada)

OK, I'd like to see if everything goes well. Merge it.

とりあえずの速報ですが,
test-all では下記のエラー:

Running tests:

[ 846/11367] PPTestModule::PPInspectTest#test_to_s_with_iv = 0.00 s

  1. Error:
    test_to_s_with_iv(PPTestModule::PPInspectTest):
    RuntimeError: can't modify frozen Float
    /mnt/sdb1/ruby/trunk/test/test_pp.rb:111:in block in test_to_s_with_iv' /mnt/sdb1/ruby/trunk/test/test_pp.rb:111:in instance_eval'
    /mnt/sdb1/ruby/trunk/test/test_pp.rb:111:in `test_to_s_with_iv'

[ 2709/11367] TestClass#test_singleton_class = 0.00 s
2) Failure:
test_singleton_class(TestClass) [/mnt/sdb1/ruby/trunk/test/ruby/test_class.rb:195]:
Exception raised:
<#<TypeError: can't define singleton>>.

[ 4022/11367] TestFloat#test_singleton_method = 0.00 s
3) Failure:
test_singleton_method(TestFloat) [/mnt/sdb1/ruby/trunk/test/ruby/test_float.rb:608]:
[TypeError] exception expected, not
Class:
Message: <"can't modify frozen Float">
---Backtrace---
/mnt/sdb1/ruby/trunk/test/ruby/test_float.rb:608:in `block in test_singleton_method'

[ 6113/11367] TestMarshal#test_float_extend = 0.00 s
4) Error:
test_float_extend(TestMarshal):
TypeError: can't define singleton
/mnt/sdb1/ruby/trunk/test/ruby/marshaltestlib.rb:224:in extend_object' /mnt/sdb1/ruby/trunk/test/ruby/marshaltestlib.rb:224:in extend'
/mnt/sdb1/ruby/trunk/test/ruby/marshaltestlib.rb:224:in `test_float_extend'

[ 6115/11367] TestMarshal#test_float_ivar = 0.00 s
5) Error:
test_float_ivar(TestMarshal):
RuntimeError: can't modify frozen Float
/mnt/sdb1/ruby/trunk/test/ruby/marshaltestlib.rb:212:in block in test_float_ivar' /mnt/sdb1/ruby/trunk/test/ruby/marshaltestlib.rb:212:in instance_eval'
/mnt/sdb1/ruby/trunk/test/ruby/marshaltestlib.rb:212:in `test_float_ivar'

[ 6116/11367] TestMarshal#test_float_ivar_self = 0.00 s
6) Error:
test_float_ivar_self(TestMarshal):
RuntimeError: can't modify frozen Float
/mnt/sdb1/ruby/trunk/test/ruby/marshaltestlib.rb:218:in block in test_float_ivar_self' /mnt/sdb1/ruby/trunk/test/ruby/marshaltestlib.rb:218:in instance_eval'
/mnt/sdb1/ruby/trunk/test/ruby/marshaltestlib.rb:218:in `test_float_ivar_self'

Finished tests in 563.933874s, 20.1566 tests/s, 6086.8360 assertions/s.
11367 tests, 3432573 assertions, 2 failures, 4 errors, 29 skips

test-ruby spec のほうでは

String#% taints result for %s when argument is tainted ERROR
RuntimeError: can't modify frozen Float
/mnt/sdb1/ruby/trunk/spec/ruby spec/core/string/modulo_spec.rb:654:in
taint' /mnt/sdb1/ruby/trunk/spec/ruby spec/core/string/modulo_spec.rb:654:in block (2 levels) in <top (required)>'
/mnt/sdb1/ruby/trunk/spec/ruby spec/core/string/modulo_spec.rb:4:in
`<top (required)>'

というエラーが出ました.

とりあえず,テストを直せば良さそうだなぁと思っています.

--
// SASADA Koichi at atdot dot net

Updated by ko1 (Koichi Sasada) over 10 years ago

(2012/10/27 10:02), SASADA Koichi wrote:

とりあえず,テストを直せば良さそうだなぁと思っています.

直しながら考えてたんですが,現在

  • 1 や :sym などは "can't define singleton method (TypeError)"
  • frozen object は "can't modify frozen Float (RuntimeError)" のよう
    に,RuntimeError

になります.

なるせさんのパッチでは,「Float の場合は TypeError」としようとしているん
ですが,これを frozen だったら TypeError にするべきか,どうか迷っており
ます.

frozen は type じゃないから,従来通り RuntimeError で良いですかね?
Fixnum など,および frozen object なら TypeError というのもアリかとふと
思ったのでした.

(Float で分岐を分けると,Bignum も frozen にしようって議論が出てきたと
き,じゃあここをどうする,とまた悩みそうだと思った次第です)

 とりあえず,なるせさんのパッチにちょっと足して,テスト修正して通ること
を確認しました.

Index: include/ruby/ruby.h

--- include/ruby/ruby.h (revision 37336)
+++ include/ruby/ruby.h (working copy)
@@ -1133,7 +1133,7 @@ struct RBignum {
(FL_TAINT | FL_UNTRUSTED);
} while (0)

-#define OBJ_FROZEN(x) (!!FL_TEST((x), FL_FREEZE))
+#define OBJ_FROZEN(x) (!!(FL_ABLE(x)?(RBASIC(x)->flags&(FL_FREEZE)):FLONUM_P(x)))
#define OBJ_FREEZE(x) FL_SET((x), FL_FREEZE)

#if SIZEOF_INT < SIZEOF_LONG
Index: class.c

--- class.c (revision 37336)
+++ class.c (working copy)
@@ -1322,6 +1322,10 @@ singleton_class_of(VALUE obj)
rb_bug("unknown immediate %p", (void *)obj);
return klass;
}

  • else {

  •   if (BUILTIN_TYPE(obj) == T_FLOAT)
    
  •       rb_raise(rb_eTypeError, "can't define singleton");
    
  • }

    if (FL_TEST(RBASIC(obj)->klass, FL_SINGLETON) &&
    rb_ivar_get(RBASIC(obj)->klass, id_attached) == obj) {
    Index: numeric.c
    ===================================================================
    --- numeric.c (revision 37336)
    +++ numeric.c (working copy)
    @@ -620,6 +620,7 @@ rb_float_new_in_heap(double d)
    NEWOBJ_OF(flt, struct RFloat, rb_cFloat, T_FLOAT);

    flt->float_value = d;

  • OBJ_FREEZE(flt);
    return (VALUE)flt;
    }

Index: vm.c

--- vm.c (revision 37336)
+++ vm.c (working copy)
@@ -1872,7 +1872,7 @@ vm_define_method(rb_thread_t *th, VALUE
}

 if (is_singleton) {
  • if (FIXNUM_P(obj) || FLONUM_P(obj) || SYMBOL_P(obj)) {
  • if (FIXNUM_P(obj) || SYMBOL_P(obj) || CLASS_OF(obj) == rb_cFloat) {
    rb_raise(rb_eTypeError,
    "can't define singleton method "%s" for %s",
    rb_id2name(id), rb_obj_classname(obj));
    Index: test/ruby/marshaltestlib.rb
    ===================================================================
    --- test/ruby/marshaltestlib.rb (revision 37336)
    +++ test/ruby/marshaltestlib.rb (working copy)
    @@ -207,30 +207,6 @@ module MarshalTestLib
    marshal_equal(NegativeZero) {|o| 1.0/o}
    end
  • def test_float_ivar

  • o1 = 1.23

  • o1.instance_eval { @iv = 1 }

  • marshal_equal(o1) {|o| o.instance_eval { @iv }}

  • end

  • def test_float_ivar_self

  • o1 = 5.5

  • o1.instance_eval { @iv = o1 }

  • marshal_equal(o1) {|o| o.instance_eval { @iv }}

  • end

  • def test_float_extend

  • o1 = 0.0/0.0

  • o1.extend(Mod1)

  • marshal_equal(o1) { |o|

  •  (class << self; self; end).ancestors
    
  • }

  • o1.extend(Mod2)

  • marshal_equal(o1) { |o|

  •  (class << self; self; end).ancestors
    
  • }

  • end

  • class MyRange < Range; def initialize(v, *args) super(*args); @v = v; end end
    def test_range
    marshal_equal(1..2)
    Index: test/ruby/test_class.rb
    ===================================================================
    --- test/ruby/test_class.rb (revision 37336)
    +++ test/ruby/test_class.rb (working copy)
    @@ -187,12 +187,8 @@ class TestClass < Test::Unit::TestCase

    def test_singleton_class
    assert_raise(TypeError) { 1.extend(Module.new) }

  • if 1.0.equal? 1.0 # flonum?
    assert_raise(TypeError) { 1.0.extend(Module.new) }

  • else

  •  assert_nothing_raised { 1.0.extend(Module.new) }
    
  • end

  • assert_nothing_raised { (2.0**1000).extend(Module.new) }

  • assert_raise(TypeError) { (2.0**1000).extend(Module.new) }
    assert_raise(TypeError) { :foo.extend(Module.new) }

    assert_in_out_err([], <<-INPUT, %w(:foo :foo true true), [])
    Index: test/test_pp.rb
    ===================================================================
    --- test/test_pp.rb (revision 37336)
    +++ test/test_pp.rb (working copy)
    @@ -107,10 +107,6 @@ class PPInspectTest < Test::Unit::TestCa
    a.instance_eval { @a = nil }
    result = PP.pp(a, '')
    assert_equal("#{a.inspect}\n", result)

  • a = 1.0
  • a.instance_eval { @a = nil }
  • result = PP.pp(a, '')
  • assert_equal("#{a.inspect}\n", result)
    end

def test_to_s_without_iv

 rubyspec のほうはまだ見ておりません.コメント頂ければ幸いです.

--
// SASADA Koichi at atdot dot net

Actions #14

Updated by ko1 (Koichi Sasada) over 10 years ago

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

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


  • numeric.c (rb_float_new_in_heap), include/ruby/ruby.h:
    make all Float objects frozen.
    [ruby-dev:46081] [ruby-trunk - Feature #6936]
    Most part of patch by NARUSE, Yui .
  • class.c (singleton_class_of): raise TypeError when
    trying to define a singleton method on Float objects.
  • vm.c (vm_define_method): ditto.
  • test/ruby/marshaltestlib.rb: catch up above changes.
  • test/ruby/test_class.rb: ditto.
  • test/test_pp.rb: ditto.

Updated by ko1 (Koichi Sasada) over 10 years ago

(2012/09/05 11:09), naruse (Yui NARUSE) wrote:

以下のような感じですかね

このパッチで気づいたんですが(これをあてると),

p 1.frozen? #=> false
p 1.0.frozen? #=> true

のように,Float は frozen なんですが,Fixnum は frozen じゃないんですね.

--
// SASADA Koichi at atdot dot net

Updated by ko1 (Koichi Sasada) over 10 years ago

(2012/10/27 10:45), SASADA Koichi wrote:

 rubyspec のほうはまだ見ておりません.コメント頂ければ幸いです.

 ("%s" % -0.0.taint).tainted?.should == true # -0.0 is not flonum

こういうテストなんですが,消せば良いんではないかと思います.

-0.0 は flonum じゃない,ってのは ad-hoc 過ぎる気がしますが....

--
// SASADA Koichi at atdot dot net

Updated by naruse (Yui NARUSE) over 10 years ago

ko1 (Koichi Sasada) wrote:

(2012/10/27 10:45), SASADA Koichi wrote:

 rubyspec のほうはまだ見ておりません.コメント頂ければ幸いです.

 ("%s" % -0.0.taint).tainted?.should == true # -0.0 is not flonum

こういうテストなんですが,消せば良いんではないかと思います.

-0.0 は flonum じゃない,ってのは ad-hoc 過ぎる気がしますが....

それも含めて現状に合わせておきました。

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0