Feature #6936

Forbid singleton class and instance variabls for float

Added by Yui NARUSE almost 3 years ago. Updated almost 3 years ago.

[ruby-dev:46081]
Status:Closed
Priority:Normal
Assignee:Koichi Sasada

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

Related to Ruby trunk - Feature #6763: Introduce Flonum technique to speedup floating computation on th 64bit environment Closed 07/21/2012
Related to Ruby trunk - Feature #3222: Can bignums have singleton class & methods? Closed 04/30/2010

Associated revisions

Revision 37341
Added by Koichi Sasada almost 3 years ago

  • numeric.c (rb_float_new_in_heap), include/ruby/ruby.h: make all Float objects frozen. [ruby-trunk - Feature #6936] Most part of patch by NARUSE, Yui naruse@ruby-lang.org.
  • 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.

Revision 37341
Added by Koichi Sasada almost 3 years ago

  • numeric.c (rb_float_new_in_heap), include/ruby/ruby.h: make all Float objects frozen. [ruby-trunk - Feature #6936] Most part of patch by NARUSE, Yui naruse@ruby-lang.org.
  • 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.

History

#1 Updated by Koichi Sasada almost 3 years ago

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

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

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

--
// SASADA Koichi at atdot dot net

#2 Updated by Motohiro KOSAKI almost 3 years ago

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

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

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

#3 Updated by Yui NARUSE almost 3 years ago

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

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

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

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

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

--
NARUSE, Yui naruse@airemix.jp

#4 Updated by Usaku NAKAMURA almost 3 years ago

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

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

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

+1

それでは。
--
U.Nakamura usa@garbagecollect.jp

#5 Updated by Kenta Murata almost 3 years ago

むらたです。

2012/8/28 U.Nakamura usa@garbagecollect.jp:

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

In message " Re: [ruby-trunk - Feature #6936][Assigned] Forbid singleton class and instance variabls for float"
on Aug.27,2012 15:22:07, ko1@atdot.net 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: mrkn@mrkn.jp
twitter: http://twitter.com/mrkn/
blog: http://d.hatena.ne.jp/mrkn/

#6 Updated by Anonymous almost 3 years ago

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

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

むらたです。

2012/8/28 U.Nakamura usa@garbagecollect.jp:

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

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

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

+1

わたしも +1。

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

すみません。
--
 稲葉 浩人 inaba@st.rim.or.jp

#7 Updated by Motohiro KOSAKI almost 3 years ago

+1

わたしも +1。

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

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

#8 Updated by Koichi Sasada almost 3 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

#9 Updated by Yui NARUSE almost 3 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; }

#10 Updated by Yukihiro Matsumoto almost 3 years ago

  • Assignee changed from Yukihiro Matsumoto to Koichi Sasada

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

Matz.

#11 Updated by Koichi Sasada almost 3 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

#12 Updated by Koichi Sasada almost 3 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:
<#>.

[ 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 のほうでは

1)
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 '
/mnt/sdb1/ruby/trunk/spec/ruby spec/core/string/modulo_spec.rb:4:in
`'

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

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

--
// SASADA Koichi at atdot dot net

#13 Updated by Koichi Sasada almost 3 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

#14 Updated by Koichi Sasada almost 3 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-trunk - Feature #6936] Most part of patch by NARUSE, Yui naruse@ruby-lang.org.
  • 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.

#15 Updated by Koichi Sasada almost 3 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

#16 Updated by Koichi Sasada almost 3 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

#17 Updated by Yui NARUSE almost 3 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 過ぎる気がしますが....

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

Also available in: Atom PDF