[Cython] Problems with decorated methods in cdef classes

[Robert Bradshaw]

On Sun, Aug 14, 2011 at 8:02 AM, Stefan Behnel <stefan_ml at behnel.de> wrote:
> Hi,
>
> I've taken another stab at #593 and changed the way decorators are currently
> evaluated.
>
> http://trac.cython.org/cython_trac/ticket/593
>
> https://github.com/cython/cython/commit/c40ff48f84b5e5841e4e2d2c6dcce3e6494e4c25
>
> We previously had
>
>    @deco
>    def func(): pass
>
> turn into this:
>
>    def func(): pass
>    func = deco(func)
>
> Note how this binds the name more than once and even looks it up in between,
> which is problematic in class scopes and some other special cases. For
> example, this doesn't work:
>
> class Foo(object):
>    @property
>    def x(self):
>        ...
>    @x.setter
>    def x(self, value):
>        ...
>
> because "x.setter" is looked up *after* binding the second function "x" to
> its method name, thus overwriting the initial property.
>
> The correct way to do it is to create the function object in a temp, pass it
> through the decorator call chain, and then assign whatever result this
> produces to the method name. This works nicely, but it triggered a crash in
> problematic code of another test case, namely "closure_decorators_T478".
> That test does this:
>
> def print_args(func):
>    def f(*args, **kwds):
>        print "args", args, "kwds", kwds
>        return func(*args, **kwds)
>    return f
>
> cdef class Num:
>    @print_args
>    def is_prime(self, bint print_factors=False):
>        ...
>
> Now, the problem is that Cython considers "is_prime" to be a method of a
> cdef class, although it actually is not. It's only an arbitrary function
> that happens to be defined inside of a cdef class body and that happens to
> be *called* by a method, namely "f". It now crashes for me because the
> "self" argument is not being passed into is_prime() as a C method argument
> when called by the wrapper function - and that's correct, because it's not a
> method call but a regular function call at that point.
>
> The correct way to fix this is to turn all decorated methods in cdef classes
> into plain functions. However, this has huge drawbacks, especially that the
> first argument ('self') can no longer be typed as the surrounding extension
> type. But, after all, you could do this:
>
> def swap_args(func):
>    def f(*args):
>        return func(*args[::-1])
>    return f
>
> cdef class Num:
>    @swap_args
>    def is_prime(arg, self):
>        ...
>
> I'm not sure what to make of this. Does it make sense to go this route? Or
> does anyone see a way to make this "mostly" work, e.g. by somehow
> restricting cdef classes and their methods? Or should we just add runtime
> checks to prevent bad behaviour of decorators?

I would be happy in making decorated methods into "ordinary"
functions--this will probably play more nicely with many real-world
decorators as well. I say we leave the arguments of a decorated method
completely untyped--the user can type the first argument if need be
(inserting the appropriate runtime check).

Now that we support decorators, perhaps the extra static/classmethod
magic could go away (or at least in the case that there are other
decorators). i suppose CyFunction could have a bit set to emulate
these behaviors for efficiency purposes, but only if it's the
immediate decorator.
(As a side note, it could be valuable and probably not too hard to
support these decorators on cdef methods as well. I'm thinking in
particular of static create* methods that take raw C data.)

- Robert