WIP ENH: allow @property decorator on external ctypedef classes

Cython/Compiler/Nodes.py

@@ -2319,7 +2319,8 @@ class CFuncDefNode(FuncDefNode):
     #  is_static_method whether this is a static method
     #  is_c_class_method whether this is a cclass method
 
-    child_attrs = ["base_type", "declarator", "body", "py_func_stat"]
+    child_attrs = ["base_type", "declarator", "body", "py_func_stat", "decorators"]
+    outer_attrs = ["decorators"]
 
     inline_in_pxd = False
     decorators = None
@@ -2339,6 +2340,16 @@ class CFuncDefNode(FuncDefNode):
         return self.py_func.code_object if self.py_func else None
 
     def analyse_declarations(self, env):
+        if self.decorators:
+            for decorator in self.decorators:
+                func = decorator.decorator
+                if func.is_name:
+                    if func.name == 'classmethod' or func.name == 'staticmethod':
+                        error(self.pos, "Cannot handle these decorators yet")
+                    if func.name == 'property':
+                        # XXX DO SOMETHING HERE???
+                        pass
+
         self.is_c_class_method = env.is_c_class_scope
         if self.directive_locals is None:
             self.directive_locals = {}
@@ -5028,6 +5039,13 @@ class PropertyNode(StatNode):
         self.entry = env.declare_property(self.name, self.doc, self.pos)
         self.entry.scope.directives = env.directives
         self.body.analyse_declarations(self.entry.scope)
+        # XXX DO SOMETHING HERE???
+        if 0 and self.is_wrapper:
+            entry = self.body.stats[0].entry
+            entry.is_property = 1
+            entry.doc = self.doc
+            env.property_entries[-1] = entry
+            env.entries[self.name] = entry
 
     def analyse_expressions(self, env):
         self.body = self.body.analyse_expressions(env)

Cython/Compiler/ParseTreeTransforms.py

@@ -1031,7 +1031,7 @@ class InterpretCompilerDirectives(CythonTransform):
             else:
                 realdecs.append(dec)
         if realdecs and (scope_name == 'cclass' or
-                         isinstance(node, (Nodes.CFuncDefNode, Nodes.CClassDefNode, Nodes.CVarDefNode))):
+                         isinstance(node, (Nodes.CClassDefNode, Nodes.CVarDefNode))):
             raise PostParseError(realdecs[0].pos, "Cdef functions/classes cannot take arbitrary decorators.")
         node.decorators = realdecs[::-1] + both[::-1]
         # merge or override repeated directives
@@ -1398,6 +1398,67 @@ class DecoratorTransform(ScopeTrackingTransform, SkipDeclarations):
         node.decorators = None
         return self.chain_decorators(node, decs, node.name)
 
+    def visit_CFuncDefNode(self, node):
+        scope_type = self.scope_type
+        if scope_type != 'cclass' or not node.decorators:
+            return node
+
+        # XXX currently only handle getter property
+        # transform @property decorators
+        properties = self._properties[-1]
+        for decorator_node in node.decorators[::-1]:
+            decorator = decorator_node.decorator
+            if decorator.is_name and decorator.name == 'property':
+                if len(node.decorators) > 1:
+                    return self._reject_decorated_property(node, decorator_node)
+                name = node.declarator.base.name
+                # XXX Disables handling property decorator
+                # return [node]
+                node.name = name #EncodedString('__get__')
+                node.decorators.remove(decorator_node)
+                stat_list = [node]
+                if name in properties:
+                    prop = properties[name]
+                    prop.pos = node.pos
+                    prop.doc = node.doc
+                    prop.body.stats = stat_list
+                    return []
+                prop = Nodes.PropertyNode(node.pos, name=name)
+                prop.doc = node.doc
+                prop.body = Nodes.StatListNode(node.pos, stats=stat_list)
+                prop.is_wrapper = True
+                properties[name] = prop
+                return [prop]
+            elif decorator.is_attribute and decorator.obj.name in properties:
+                # TODO fix this
+                raise error(decorator_node.pos, "Not handled yet")
+                handler_name = self._map_property_attribute(decorator.attribute)
+                if handler_name:
+                    if decorator.obj.name != node.name:
+                        # CPython generates neither an error nor warning, but nothing useful either.
+                        error(decorator_node.pos,
+                              "Mismatching property names, expected '%s', got '%s'" % (
+                                  decorator.obj.name, node.name))
+                    elif len(node.decorators) > 1:
+                        return self._reject_decorated_property(node, decorator_node)
+                    else:
+                        return self._add_to_property(properties, node, handler_name, decorator_node)
+
+        # we clear node.decorators, so we need to set the
+        # is_staticmethod/is_classmethod attributes now
+        for decorator in node.decorators:
+            # TODO fix this
+            raise error(decorator.pos, "Not handled yet")
+            func = decorator.decorator
+            if func.is_name:
+                node.is_classmethod |= func.name == 'classmethod'
+                node.is_staticmethod |= func.name == 'staticmethod'
+
+        # transform normal decorators
+        decs = node.decorators
+        node.decorators = None
+        return self.chain_decorators(node, decs, None)
+
     @staticmethod
     def _reject_decorated_property(node, decorator_node):
         # restrict transformation to outermost decorator as wrapped properties will probably not work

tests/run/ext_attr_getter.srctree

@@ -4,14 +4,21 @@ PYTHON -c "import runner"
 ######## setup.py ########
 
 from Cython.Build.Dependencies import cythonize
+from Cython.Compiler.Errors import CompileError
 from distutils.core import setup
 
 # force the build order
-setup(ext_modules= cythonize("foo_extension.pyx"))
+setup(ext_modules= cythonize("foo_extension.pyx", language_level=3))
 
-setup(ext_modules = cythonize("getter*.pyx"))
+setup(ext_modules = cythonize("getter[0-9].pyx", language_level=3))
 
-######## foo_nominal.h ########
+try:
+    cythonize("getter_fail0.pyx", language_level=3)
+    assert False
+except CompileError:
+    print("\nGot expected exception, continuing\n")
+
+######## foo.h ########
 
 #include <Python.h>
 
@@ -26,6 +33,30 @@ typedef struct {
     int f2;
 } FooStructNominal;
 
+typedef struct {
+    PyObject_HEAD
+} FooStructOpaque;
+
+
+#define PyFoo_GET0(a) a.f0
+#define PyFoo_GET1(a) a.f1
+#define PyFoo_GET2(a) a.f2
+
+int PyFoo_Get0(FooStructNominal f)
+{
+    return f.f0;
+}
+
+int PyFoo_Get1(FooStructNominal f)
+{
+    return f.f1;
+}
+
+int PyFoo_Get2(FooStructNominal f)
+{
+    return f.f2;
+}
+
 #ifdef __cplusplus
 }
 #endif
@@ -33,21 +64,24 @@ typedef struct {
 ######## foo_extension.pyx ########
 
 cdef class Foo:
-    cdef public int field0, field1, field2;
+    cdef public int _field0, _field1, _field2;
 
-    def __init__(self, f0, f1, f2):
-        self.field0 = f0
-        self.field1 = f1
-        self.field2 = f2
+    @property
+    def field0(self):
+        return self._field0
 
-cdef get_field0(Foo f):
-    return f.field0
+    @property
+    def field1(self):
+        return self._field1
 
-cdef get_field1(Foo f):
-    return f.field1
+    @property
+    def field2(self):
+        return self._field2
 
-cdef get_field2(Foo f):
-    return f.field2
+    def __init__(self, f0, f1, f2):
+        self._field0 = f0
+        self._field1 = f1
+        self._field2 = f2
 
 # A pure-python class that disallows direct access to fields
 class OpaqueFoo(Foo):
@@ -64,12 +98,11 @@ class OpaqueFoo(Foo):
     def field2(self):
         raise AttributeError('no direct access to field2')
 
-
 ######## getter0.pyx ########
 
 # Access base Foo fields from C via aliased field names
 
-cdef extern from "foo_nominal.h":
+cdef extern from "foo.h":
 
     ctypedef class foo_extension.Foo [object FooStructNominal]:
         cdef:
@@ -78,13 +111,62 @@ cdef extern from "foo_nominal.h":
             int field2 "f2"
 
 def sum(Foo f):
-    # the f.__getattr__('field0') is replaced in c by f->f0
+    # Note - not a cdef function but compiling the f.__getattr__('field0')
+    # notices the alias and replaces the __getattr__ in c by f->f0 anyway
     return f.field0 + f.field1 + f.field2
 
+######## getter1.pyx ########
+
+# Access base Foo fields from C via getter functions
+
+
+cdef extern from "foo.h":
+    ctypedef class foo_extension.Foo [object FooStructOpaque]:
+    # Importing will warn until we can use this syntax
+    # ctypedef class foo_extension.Foo [object FooStructOpaque, check_size]:
+        @property
+        cdef int field0(self):
+            return PyFoo_GET0(self)
+
+        @property
+        cdef int field1(self):
+            return PyFoo_Get1(self)
+
+        @property
+        cdef int field2(self):
+            return PyFoo_GET2(self)
+
+    int PyFoo_GET0(Foo);  # this is actually a macro !
+    int PyFoo_Get1(Foo);
+    int PyFoo_GET2(Foo);  # this is actually a macro !
+
+def sum(Foo f):
+    # Note - not a cdef function but compiling the f.__getattr__('field0')
+    # notices the getter and replaces the __getattr__ in c by PyFoo_GET anyway
+    return f.field0 + f.field1 + f.field2
+
+
+######## getter_fail0.pyx ########
+
+# Make sure not all decorators are accepted
+
+cdef extern from "foo.h":
+    ctypedef class foo_extension.Foo [object FooStructOpaque]:
+        @staticmethod
+        cdef void field0():
+            print('in staticmethod of Foo')
+
+
 ######## runner.py ########
 
-import foo_extension, getter0
+import warnings
+import foo_extension, getter0, getter1
+
+def sum(f):
+    # pure python field access, but code is identical to cython cdef sum
+    return f.field0 + f.field1 + f.field2
 
+# Baseline test: if this fails something else is wrong
 foo = foo_extension.Foo(23, 123, 1023)
 
 assert foo.field0 == 23
@@ -92,18 +174,31 @@ assert foo.field1 == 123
 assert foo.field2 == 1023
 
 ret =  getter0.sum(foo)
-assert ret == foo.field0 + foo.field1 + foo.field2
+assert ret == sum(foo)
+
+# Aliasing test. Check 'cdef int field0 "f0" works as advertised:
+# - C can access the fields through the aliases
+# - Python cannot access the fields at all
 
 opaque_foo = foo_extension.OpaqueFoo(23, 123, 1023)
 
-# C can access the fields through the aliases
 opaque_ret = getter0.sum(opaque_foo)
 assert opaque_ret == ret
 try:
-    # Python cannot access the fields
     f0 = opaque_ret.field0
     assert False
 except AttributeError as e:
     pass
 
+# Getter test. Check C-level getter works as advertised:
+# - C accesses the fields through getter calls (maybe macros)
+# - Python accesses the fields through attribute lookup
+
+opaque_foo = foo_extension.OpaqueFoo(23, 123, 1023)
+
+# Remove warnings filter once we can use check_size=False
+with warnings.catch_warnings():
+    warnings.simplefilter("ignore")
+    opaque_ret = getter1.sum(opaque_foo)
+assert opaque_ret == ret