Feature #6936: Forbid singleton class and instance variabls for float

• Status: Closed
• Priority: Normal
• Assignee: ko1 (Koichi Sasada)
• Target version: 2.0.0

[naruse (Yui NARUSE)]

[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 っぽいオブジェクトにも当てはまる気がしています。

[ko1 (Koichi Sasada)]

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

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

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

--
// SASADA Koichi at atdot dot net

[kosaki (Motohiro KOSAKI)]

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

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

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

[naruse (Yui NARUSE)]

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

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

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

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

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

--
NARUSE, Yui [e-mail]

[usa (Usaku 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, [e-mail] wrote:

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

+1

それでは。
--
U.Nakamura [e-mail]

[mrkn (Kenta Murata)]

むらたです。

2012/8/28 U.Nakamura [e-mail]:

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

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, [e-mail] 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: [e-mail]
twitter: http://twitter.com/mrkn/
blog: http://d.hatena.ne.jp/mrkn/

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

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

むらたです。

2012/8/28 U.Nakamura [e-mail]:

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

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, [e-mail] wrote:

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

+1

わたしも +1。

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

すみません。
--
　稲葉 浩人 [e-mail]

[kosaki (Motohiro KOSAKI)]

+1

わたしも +1。

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

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

[ko1 (Koichi Sasada)]

(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

[naruse (Yui NARUSE)]

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; }

[matz (Yukihiro Matsumoto)]

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

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

Matz.

[ko1 (Koichi Sasada)]

(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

[ko1 (Koichi Sasada)]

(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:ininstance_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:inextend'
/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:ininstance_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:ininstance_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

[ko1 (Koichi Sasada)]

(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

[ko1 (Koichi Sasada)]

• 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 [e-mail].
• 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.

[ko1 (Koichi Sasada)]

(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

[ko1 (Koichi Sasada)]

(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

[naruse (Yui NARUSE)]

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 過ぎる気がしますが...．

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