From: Trans <transfire@...>
Date: 2008-01-24T07:07:10+09:00
Subject: Re: Bugs in Facets Cloneable?



On Jan 22, 7:39 pm, Ken Bloom <kbl...@gmail.com> wrote:
> On Tue, 22 Jan 2008 17:14:55 -0500, Trans wrote:
> > On Jan 22, 11:09 am, Ken Bloom <kbl...@gmail.com> wrote:
> >> I was just looking at the Cloneable module from the Facets' library,
> >> because it's useful, and noticed a couple of possible bugs. Is my
> >> assessment of this correct?
>
> >> module Cloneable
> >>   def clone
> >>     sibling = self.class.new
> >>     instance_variables.each do |ivar|
> >>       value = self.instance_variable_get(ivar)
> >>       sibling.instance_variable_set(ivar, value.dup) #rake_dup)
> >>     end
> >>     sibling
> >>   end
> >>   alias_method :dup, :clone
> >> end
>
> >> Question 1: Shouldn't #clone use self.class.allocate instead of
> >> self.class.new? Better yet, shouldn't the superclass's dup be called
> >> somehow?
>
> >> Question 2: Shouldn't #clone and #dup be implemented differently, so
> >> that #clone can freeze the newly created object?
>
> > Hmmm.. some good questions. This bit of code came from Rake a long time
> > ago. I never really analysed it, and just accepted it at face value. I
> > suspect the thing to do these days is to use #allocate and
> > #initialize_copy instead.
>
> > I point out, if anyone is wondering, the difference between this and the
> > built-in clone/dup is that the above dups the instance vars.
>
> > The difference between the two being that #dup carries over the tainted
> > state while #clone carries over the tainted and frozen states
> > (correct?). Since we are duplicating the instance vars here, there is no
> > reason to consider taintedness. But frozen should still apply (yes?).
>
> Basically, I think it should be possible for an object to encapsulate
> (and hide) member variables that should be copied (not shared) when the
> enclosing object is copied. Thus, a semi-deep copying version of #clone
> and #dup are in order. That seems to be what Cloneable is about.
>
> Thanks for pointing out #initialize_copy. I didn't know it existed. Now I
> see it in PickAxe, but I think it's role has been greatly trivialized.
> Pickaxe only mentions it with regard to C extensions, empathetically says
> that it's only for C extensions, and completely overlooks the idea that
> an object might be hiding other kinds of information that should be
> copied, not shared between duplicates.
>
> Might I suggest that instead of overriding #dup and #clone at all,
> Cloneable should just provide the following implementation for
> #initialize_copy, then we can make #dup and #clone will both behave
> properly by virtue of the fact that they descend from Object#dup and
> Object#clone:
>
> module Cloneable
>   def initialize_copy sibling
>     #first duplicate my superclass' state. Note that if it's duplicating
>     #instance variables, this will be overwritten, but this is important
>     #because we could be dealing with a C extension with state hidden from
>     #the Ruby interpreter
>     super

Nice. Makes sense to me. I will work on this a bit and incorporate --
with due credit.

Thanks Ken,
T.