Fixing up flawed fix for #303

Cython/Compiler/Nodes.py

@@ -794,21 +794,25 @@ class CVarDefNode(StatNode):
         self.dest_scope = dest_scope
         base_type = self.base_type.analyse(env)
 
+        need_property = False
+        if (dest_scope.is_c_class_scope
+              and self.visibility == 'public'
+              and base_type.is_pyobject
+              and (base_type.is_builtin_type or base_type.is_extension_type)):
+            # If the field is settable and extension type, then the CPython mechanism does
+            # not do enough type-checking for us.
+            need_property = True
+        elif (base_type.is_typedef and base_type.typedef_is_external
+              and (self.visibility in ('public', 'readonly'))):
             # If the field is an external typedef, we cannot be sure about the type,
             # so do conversion ourself rather than rely on the CPython mechanism (through
             # a property; made in AnalyseDeclarationsTransform).
-        # Also, if the type is an extension type, then the CPython mechanism does
-        # not do enough type-checking for us.
-        if (dest_scope.is_c_class_scope and
-               ((self.visibility == 'public' 
-                 and base_type.is_pyobject 
-                 and (base_type.is_builtin_type or base_type.is_extension_type)
-                or (base_type.is_typedef and base_type.typedef_is_external)))):
-            self.need_properties = []
             need_property = True
+
+        if need_property:
             visibility = 'private'
+            self.need_properties = []
         else:
-            need_property = False
             visibility = self.visibility
             
         for declarator in self.declarators:

Cython/Compiler/ParseTreeTransforms.py

@@ -676,6 +676,12 @@ property NAME:
         ATTR = value
     """, level='c_class')
 
+    readonly_property = TreeFragment(u"""
+property NAME:
+    def __get__(self):
+        return ATTR
+    """, level='c_class')
+    
     def __call__(self, root):
         self.env_stack = [root.scope]
         # needed to determine if a cdef var is declared after it's used.
@@ -752,7 +758,7 @@ property NAME:
             # mechanism for them. 
             stats = []
             for entry in node.need_properties:
-                property = self.create_Property(entry)
+                property = self.create_Property(entry, node.visibility == 'readonly')
                 property.analyse_declarations(node.dest_scope)
                 self.visit(property)
                 stats.append(property)
@@ -760,8 +766,12 @@ property NAME:
         else:
             return None
             
-    def create_Property(self, entry):
-        property = self.basic_property.substitute({
+    def create_Property(self, entry, readonly):
+        if readonly:
+            template = self.readonly_property
+        else:
+            template = self.basic_property
+        property = template.substitute({
                 u"ATTR": AttributeNode(pos=entry.pos,
                                        obj=NameNode(pos=entry.pos, name="self"), 
                                        attribute=entry.name),

tests/run/typedfieldbug_T303.pyx

 """
 >>> f()
-42.0 42.0
+42.0 42.0 42.0
+>>> readonly()
+Traceback (most recent call last):
+    ...
+AttributeError: attribute 'var_nf' of 'typedfieldbug_T303.MyClass' objects is not writable
 """
 
 cdef extern from "external_defs.h":
@@ -10,11 +14,18 @@ cdef class MyClass:
     cdef readonly:
         double var_d
         DoubleTypedef var_nf
+    cdef public:
+        DoubleTypedef mutable
     def __init__(self):
         self.var_d = 42.0
         self.var_nf = 42.0
+        self.mutable = 1
 
 def f():
     c = MyClass()
-    print c.var_d, c.var_nf
+    c.mutable = 42.0
+    print c.var_d, c.var_nf, c.mutable
 
+def readonly():
+    c = MyClass()
+    c.var_nf = 3