[Cython] Problems with decorated methods in cdef classes

Stefan Behnel stefan_ml at behnel.de
Sun Aug 14 19:15:27 CEST 2011


Vitja Makarov, 14.08.2011 18:55:
> 2011/8/14 Stefan Behnel<stefan_ml at behnel.de>:
>> 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.
>
> I'd prefer removal of assignment synthesis from DefNode and
> transformation that injects assignment node.
> I hope that could make code cleaner.

Absolutely.


>> 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?
>
> Future CyFunction optimizaition could help here. CyFunction should
> accept the following two signatures:
>
>   - self is passed as second (or first?) C-function parameter
>   - self is passed inside tuple args

Yes, it would simply take the 'self' argument from the Python arguments, 
check its type and raise an exception if it doesn't match the expected 
type. That's what I meant with "runtime checks". This does sound like a 
reasonable compromise between static extension type semantics and general 
Python semantics. After all, if users want general functions, they can 
always define them outside of the class scope.

There are other issues with decorators in cdef classes, though. For 
example, this

cdef class T:
     @staticmethod
     @some_deco
     def func(arg): pass

is supposed to be different from this:

cdef class T:
     @some_deco
     @staticmethod
     def func(arg): pass

But currently, staticmethod and classmethod change func() directly.

Stefan


More information about the cython-devel mailing list