[PATCH 2 of 2] keyword: eliminate potential reference cycles from kwrepo

Matt Mackall mpm at selenic.com
Wed Jul 8 10:57:01 CDT 2009


On Wed, 2009-07-08 at 00:43 +0200, Christian Ebert wrote:
> * Martin Geisler on Tuesday, July 07, 2009 at 22:50:37 +0200
> > Matt Mackall <mpm at selenic.com> writes:
> > 
> >> How do we work around this? Good question. In the case of kwrepo,
> >> where we're defining a new class already, the easiest thing to do is
> >> simply to superclass:
> >> 
> >> class baz(foo):
> >>  def bar(self):
> >>    # add our baz-specific behavior
> >>    ...
> >>    # fall through to base behavior where appropriate
> >>    super(baz, self).bar()
> >> 
> >> Then no reassignment is necessary and we never need to deal with bound
> >> methods.
> >
> > Right, that makes good sense. And so I would say that the part of the
> > patch that adds wrappers like this is unnecessary (since we don't add
> > any kwrepo-specific behavior to sopener):
> > 
> >    class kwrepo(repo.__class__):
> >        def sopener(self, *args):
> >            return super(kwrepo, self).sopener(*args)
> 
> In the code should it be:
> 
>     class kwrepo(repo.__class__):
>         def file(self, f):
>             if f[0] == '/':
>                 f = f[1:]
>             return kwfilelog(self.sopener, kwt, f)
> 
> or:
> 
>     class kwrepo(repo.__class__):
>         def file(self, f):
>             if f[0] == '/':
>                 f = f[1:]
>             return kwfilelog(repo.sopener, kwt, f)
>                              ^^^^
> or doesn't it matter at all?

Never use repo. That's the second issue. It's effectively the same as:

a = {}
a['a'] = a # put a reference to a inside a
print a
{'a': {...}}

-- 
http://selenic.com : development and support for Mercurial and Linux




More information about the Mercurial-devel mailing list