Bug #2679: rubyspec: SimpleDelegator when frozen creates a frozen clone ERROR

• Status: Closed
• Assignee: nobu (Nobuyoshi Nakada)
• Target version: 2.0.0
• ruby -v: ruby 1.9.2dev (2010-01-29 trunk 26468) [i686-linux]

[mame (Yusuke Endoh)]

=begin
まつもとさん、または lib/delegator.rb についてわかる誰か
遠藤です。

freeze した SimpleDelegator を clone できません。

$ ./ruby -rdelegate -e '
a = [42, :hello]
d = SimpleDelegator.new(a)
d.freeze
d.clone
'
/home/mame/work/ruby-trunk-local/lib/ruby/1.9.1/delegate.rb:257:in
__setobj__': can't modify frozen object (RuntimeError) from /home/mame/work/ruby-trunk-local/lib/ruby/1.9.1/delegate.rb:207:in clone'
from -e:5:in `'

これはバグですよね。

lib/delegator.rb の clone の定義を見ると

def clone
new = super
new.setobj(getobj.clone)
new
end

となっていて、setobj するときにはもう SimpleDelegetor は freeze
状態なのでそりゃそうだという感じなんですが、どう直したものでしょう。

--
Yusuke ENDOH [e-mail]
=end

[matz (Yukihiro Matsumoto)]

=begin
まつもと ゆきひろです

In message "Re: [ruby-dev:40221] [Bug:trunk] rubyspec: SimpleDelegator when frozen creates a frozen clone ERROR"
on Fri, 29 Jan 2010 01:18:07 +0900, Yusuke ENDOH [e-mail] writes:

|freeze した SimpleDelegator を clone できません。
|これはバグですよね。

バグだと思います。

|lib/delegator.rb の clone の定義を見ると
|
| def clone
| new = super
| new.setobj(getobj.clone)
| new
| end
|
|となっていて、setobj するときにはもう SimpleDelegetor は freeze
|状態なのでそりゃそうだという感じなんですが、どう直したものでしょう。

困りましたねえ。ちょっと考えてみます。

=end

[matz (Yukihiro Matsumoto)]

=begin
まつもと ゆきひろです

In message "Re: [ruby-dev:40222] Re: [Bug:trunk] rubyspec: SimpleDelegator when frozen creates a frozen clone ERROR"
on Fri, 29 Jan 2010 01:36:27 +0900, Yukihiro Matsumoto [e-mail] writes:

||lib/delegator.rb の clone の定義を見ると
||
|| def clone
|| new = super
|| new.setobj(getobj.clone)
|| new
|| end
||
||となっていて、setobj するときにはもう SimpleDelegetor は freeze
||状態なのでそりゃそうだという感じなんですが、どう直したものでしょう。
|
|困りましたねえ。ちょっと考えてみます。

そういえば、こんな時のためにinitialize_copyというメソッドを用
意しておいたのでした。でも、今度はcloneとdupの区別がつかない
(API設計ミス)。もし、これによって非互換性が問題になるようなら
教えてください。

=end

[matz (Yukihiro Matsumoto)]

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

=begin
This issue was solved with changeset r26462.
Yusuke, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

=end

[mame (Yusuke Endoh)]

=begin
遠藤です。

2010年1月29日1:49 Yukihiro Matsumoto [e-mail]:

||lib/delegator.rb の clone の定義を見ると
||
|| def clone
|| new = super
|| new.setobj(getobj.clone)
|| new
|| end
||
||となっていて、setobj するときにはもう SimpleDelegetor は freeze
||状態なのでそりゃそうだという感じなんですが、どう直したものでしょう。
|
|困りましたねえ。ちょっと考えてみます。

そういえば、こんな時のためにinitialize_copyというメソッドを用
意しておいたのでした。でも、今度はcloneとdupの区別がつかない
(API設計ミス)。もし、これによって非互換性が問題になるようなら
教えてください。

(いい加減に試したところ) rubyspec と test-all ともにパスしたのですが、
以下のようなコードが動かなくなりました。

$ ./ruby -rdelegate -e '
a = [42, :hello].freeze
d = SimpleDelegator.new(a)
d.dup[0] += 1
'
-e:4:in `': can't modify frozen array (RuntimeError)

--
Yusuke ENDOH [e-mail]

=end

[mame (Yusuke Endoh)]

• Category set to lib
• Status changed from Closed to Open
• Assignee set to matz (Yukihiro Matsumoto)
• Priority changed from 3 to Normal
• Target version set to 2.0.0
• ruby -v set to ruby 1.9.2dev (2010-01-29 trunk 26468) [i686-linux]

=begin
$ ./ruby -rdelegate -e '
a = [42, :hello].freeze
d = SimpleDelegator.new(a)
d.dup[0] += 1
'
-e:4:in `': can't modify frozen array (RuntimeError)

忘れられないように reopen しておきます。

--
Yusuke Endoh [e-mail]
=end

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed

=begin
This issue was solved with changeset r26566.
Yusuke, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

=end

[matz (Yukihiro Matsumoto)]

=begin
まつもと ゆきひろです

In message "Re: [ruby-dev:40312] Bug #2679 rubyspec: SimpleDelegator when frozen creates a frozen clone ERROR"
on Thu, 4 Feb 2010 08:17:45 +0900, Nobuyoshi Nakada [e-mail] writes:

|チケット #2679 が更新されました。 (by Nobuyoshi Nakada)
|ステータス OpenからClosedに変更

この修正ですが、SimpleDelegator.dupが返すオブジェクトのclass
がSimpleDelegatorでなくなってます。それはそれで問題があるよ
うな。

a = [42, :hello].freeze
d = SimpleDelegator.new(a)
d.dup.class # 以前 SimpleDelegator, 現在 Array

どうなんでしょうね。

=end

[mame (Yusuke Endoh)]

• Status changed from Closed to Open
• Assignee changed from matz (Yukihiro Matsumoto) to nobu (Nobuyoshi Nakada)

=begin
遠藤です。

rubyspec で delegate のエラーが 5 つ増えました。
tempfile も 10 個くらい失敗するようになったようです。
難しいですね。

翻訳すると、以下のように挙動が変わったようです。

1) freeze しても setobj できてしまう

$ ./ruby -rdelegate -e '
d = SimpleDelegator.new([1, :foo])
d.freeze
d.setobj("foo") # 以前はここで例外
p d
'
"foo"

2) frozen? が delegate 元の freeze 状態を返す (これはむしろ改善？)

$ ./ruby -rdelegate -e '
p SimpleDelegator.new([1, :foo].freeze).frozen?
'
true # 以前は false

3) SimpleDelegator#method の後で setobj を呼び出してから call できる

$ ./ruby -rdelegate -e '
s = SimpleDelegator.new("foo")
m = s.method("upcase")
s.setobj([1,2,3])
p m.call # 以前はここで例外
'
"FOO"

4) delegate 元の private method が普通に呼び出せてしまう (重大)

$ ./ruby -rdelegate -e '
class C
def foo
p :foo!
end
private :foo
end
SimpleDelegator.new(C.new).foo # 以前は例外
'
foo!

5) delegate 元の private method は以前は send でも呼び出せなかった

$ ./ruby -rdelegate -e '
class C
def foo
p :foo!
end
private :foo
end
SimpleDelegator.new(C.new).send(:foo) # 以前は例外
'
foo!

--
Yusuke ENDOH [e-mail]
=end

[nobu (Nobuyoshi Nakada)]

=begin
なかだです。

At Thu, 4 Feb 2010 12:43:19 +0900,
Yusuke Endoh wrote in [ruby-dev:40314]:

2) frozen? が delegate 元の freeze 状態を返す (これはむしろ改善？)

$ ./ruby -rdelegate -e '
p SimpleDelegator.new([1, :foo].freeze).frozen?
'
true # 以前は false

これはどっちのほうがいいんでしょうねぇ。

4) delegate 元の private method が普通に呼び出せてしまう (重大)

$ ./ruby -rdelegate -e '
class C
def foo
p :foo!
end
private :foo
end
SimpleDelegator.new(C.new).foo # 以前は例外
'
foo!

5) delegate 元の private method は以前は send でも呼び出せなかった

$ ./ruby -rdelegate -e '
class C
def foo
p :foo!
end
private :foo
end
SimpleDelegator.new(C.new).send(:foo) # 以前は例外
'
foo!

むしろsendでも呼べないほうがバグじゃないかという気がしますが、
method_missingではどう呼び出されたかは分からないので#4と#5で違う
動作にはできませんね。

--
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
中田 伸悦

=end

[mame (Yusuke Endoh)]

=begin
遠藤です。

2010年2月5日16:39 Nobuyoshi Nakada [e-mail]:

4) delegate 元の private method が普通に呼び出せてしまう (重大)

snip

むしろsendでも呼べないほうがバグじゃないかという気がしますが、

重大と言ったのは、[ruby-core:26122] で「public method しか delegate
しない」というまつもとさんの方針が示されているためです。
それを根拠に rubyspec を修正したのは私なのでちょっと気まずいですが、
キャンセルされるということなら再度修正します。

まつもとさん、どうします？

--
Yusuke ENDOH [e-mail]

=end

[mame (Yusuke Endoh)]

=begin
遠藤です。

2010年2月5日23:05 Yusuke ENDOH [e-mail]:

2010年2月5日16:39 Nobuyoshi Nakada [e-mail]:

4) delegate 元の private method が普通に呼び出せてしまう (重大)

snip

むしろsendでも呼べないほうがバグじゃないかという気がしますが、

重大と言ったのは、[ruby-core:26122] で「public method しか delegate
しない」というまつもとさんの方針が示されているためです。
それを根拠に rubyspec を修正したのは私なのでちょっと気まずいですが、
キャンセルされるということなら再度修正します。

上記を送ってから気がつきましたが、すでに修正していただいていました。
すみません＆ありがとうございます。

これによって、rubyspec のエラーは現在 1 つになっています。

$ ./ruby -rdelegate -e '
d = SimpleDelegator.new([42, :hello])
d.freeze
p d.clone.frozen?
'
false

上記で true が期待されています。
重要度は低そうなので、仕様変更ということならそれで構いません。

--
Yusuke ENDOH [e-mail]

=end

[matz (Yukihiro Matsumoto)]

=begin
まつもと ゆきひろです

In message "Re: [ruby-dev:40330] Re: Bug #2679 rubyspec: SimpleDelegator when frozen creates a frozen clone ERROR"
on Fri, 5 Feb 2010 23:05:03 +0900, Yusuke ENDOH [e-mail] writes:

|重大と言ったのは、[ruby-core:26122] で「public method しか delegate
|しない」というまつもとさんの方針が示されているためです。
|それを根拠に rubyspec を修正したのは私なのでちょっと気まずいですが、
|キャンセルされるということなら再度修正します。
|
|まつもとさん、どうします？

えーと、別件で来週前半までめちゃめちゃ忙しくてゆっくり考える
時間がありません。なかださんの修正もまだフォローしてないし。
というわけで、ちょっとだけ時間をください。水曜か木曜辺りから、
この件について真面目に検討します。

=end

[mame (Yusuke Endoh)]

• Status changed from Open to Closed

=begin
Hi,

This ticket was duplicated by #2679.
After that, lib/delegate.rb has been changed bit by bit.
As a result of matz and nobu's effort, this seems to work now.

--
Yusuke Endoh [e-mail]
=end