From 5a76ba3e06aa675c2da24e1553ee87061fbf1f0d Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Sat, 27 Jul 2013 07:27:46 +0200 Subject: [PATCH] prevent simple builtin typed exttype attributes from triggering GC support --- Cython/Compiler/PyrexTypes.py | 8 ++++ Cython/Compiler/Symtab.py | 26 +++++++---- Cython/Compiler/TypeSlots.py | 16 +++---- tests/run/cyclic_gc.pyx | 101 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 135 insertions(+), 16 deletions(-) create mode 100644 tests/run/cyclic_gc.pyx diff --git a/Cython/Compiler/PyrexTypes.py b/Cython/Compiler/PyrexTypes.py index 6059e36..1e169ee 100755 --- a/Cython/Compiler/PyrexTypes.py +++ b/Cython/Compiler/PyrexTypes.py @@ -859,6 +859,7 @@ class PyObjectType(PyrexType): buffer_defaults = None is_extern = False is_subclassed = False + is_gc_simple = False def __str__(self): return "Python object" @@ -909,6 +910,12 @@ class PyObjectType(PyrexType): return cname +builtin_types_that_cannot_create_refcycles = set([ + 'bool', 'int', 'long', 'float', 'complex', + 'bytearray', 'bytes', 'unicode', 'str', 'basestring' +]) + + class BuiltinObjectType(PyObjectType): # objstruct_cname string Name of PyObject struct @@ -929,6 +936,7 @@ class BuiltinObjectType(PyObjectType): self.cname = cname self.typeptr_cname = "(&%s)" % cname self.objstruct_cname = objstruct_cname + self.is_gc_simple = name in builtin_types_that_cannot_create_refcycles def set_scope(self, scope): self.scope = scope diff --git a/Cython/Compiler/Symtab.py b/Cython/Compiler/Symtab.py index 7f93f42..9c9059c 100644 --- a/Cython/Compiler/Symtab.py +++ b/Cython/Compiler/Symtab.py @@ -1769,6 +1769,7 @@ class CClassScope(ClassScope): # method_table_cname string # getset_table_cname string # has_pyobject_attrs boolean Any PyObject attributes? + # has_cyclic_pyobject_attrs boolean Any PyObject attributes that may need GC? # property_entries [Entry] # defined boolean Defined in .pxd file # implemented boolean Defined in .pyx file @@ -1776,24 +1777,30 @@ class CClassScope(ClassScope): is_c_class_scope = 1 + has_pyobject_attrs = False + has_cyclic_pyobject_attrs = False + defined = False + implemented = False + def __init__(self, name, outer_scope, visibility): ClassScope.__init__(self, name, outer_scope) if visibility != 'extern': self.method_table_cname = outer_scope.mangle(Naming.methtab_prefix, name) self.getset_table_cname = outer_scope.mangle(Naming.gstab_prefix, name) - self.has_pyobject_attrs = 0 self.property_entries = [] self.inherited_var_entries = [] - self.defined = 0 - self.implemented = 0 def needs_gc(self): # If the type or any of its base types have Python-valued # C attributes, then it needs to participate in GC. - return self.has_pyobject_attrs or \ - (self.parent_type.base_type and - self.parent_type.base_type.scope is not None and - self.parent_type.base_type.scope.needs_gc()) + if self.has_cyclic_pyobject_attrs: + return True + base_type = self.parent_type.base_type + if base_type and base_type.scope is not None: + return base_type.scope.needs_gc() + elif self.parent_type.is_builtin_type: + return not self.parent_type.is_gc_simple + return False def declare_var(self, name, type, pos, cname = None, visibility = 'private', @@ -1819,7 +1826,10 @@ class CClassScope(ClassScope): entry.is_variable = 1 self.var_entries.append(entry) if type.is_pyobject and name != '__weakref__': - self.has_pyobject_attrs = 1 + self.has_pyobject_attrs = True + if (not type.is_builtin_type + or not type.scope or type.scope.needs_gc()): + self.has_cyclic_pyobject_attrs = True if visibility not in ('private', 'public', 'readonly'): error(pos, "Attribute of extension type cannot be declared %s" % visibility) diff --git a/Cython/Compiler/TypeSlots.py b/Cython/Compiler/TypeSlots.py index 0413e4a..49eacd4 100644 --- a/Cython/Compiler/TypeSlots.py +++ b/Cython/Compiler/TypeSlots.py @@ -319,10 +319,10 @@ class GCDependentSlot(InternalMethodSlot): def slot_code(self, scope): if not scope.needs_gc(): return "0" - if not scope.has_pyobject_attrs: - # if the type does not have object attributes, it can - # delegate GC methods to its parent - iff the parent - # functions are defined in the same module + if not scope.has_cyclic_pyobject_attrs: + # if the type does not have GC relevant object attributes, it can + # delegate GC methods to its parent - iff the parent functions + # are defined in the same module parent_type_scope = scope.parent_type.base_type.scope if scope.parent_scope is parent_type_scope.parent_scope: entry = scope.parent_scope.lookup_here(scope.parent_type.base_type.name) @@ -339,10 +339,10 @@ class ConstructorSlot(InternalMethodSlot): self.method = method def slot_code(self, scope): - if self.slot_name != 'tp_new' \ - and scope.parent_type.base_type \ - and not scope.has_pyobject_attrs \ - and not scope.lookup_here(self.method): + if (self.slot_name != 'tp_new' + and scope.parent_type.base_type + and not scope.has_pyobject_attrs + and not scope.lookup_here(self.method)): # if the type does not have object attributes, it can # delegate GC methods to its parent - iff the parent # functions are defined in the same module diff --git a/tests/run/cyclic_gc.pyx b/tests/run/cyclic_gc.pyx new file mode 100644 index 0000000..ff26071 --- /dev/null +++ b/tests/run/cyclic_gc.pyx @@ -0,0 +1,101 @@ +# mode: run +# tag: cyclicgc + + +cimport cython + +@cython.test_fail_if_path_exists('//CClassDefNode[@scope.has_cyclic_pyobject_attrs = True]') +@cython.test_assert_path_exists('//CClassDefNode', + '//CClassDefNode[@scope]', + '//CClassDefNode[@scope.has_cyclic_pyobject_attrs = False]') +cdef class ExtTypeNoGC: + """ + >>> obj = ExtTypeNoGC() + >>> obj = ExtTypeNoGC() + >>> obj = ExtTypeNoGC() + >>> obj = ExtTypeNoGC() + >>> obj = ExtTypeNoGC() + >>> obj = ExtTypeNoGC() + """ + + +@cython.test_fail_if_path_exists('//CClassDefNode[@scope.has_cyclic_pyobject_attrs = True]') +@cython.test_assert_path_exists('//CClassDefNode', + '//CClassDefNode[@scope]', + '//CClassDefNode[@scope.has_cyclic_pyobject_attrs = False]') +cdef class ExtSubTypeNoGC(ExtTypeNoGC): + """ + >>> obj = ExtSubTypeNoGC() + >>> obj = ExtSubTypeNoGC() + >>> obj = ExtSubTypeNoGC() + >>> obj = ExtSubTypeNoGC() + >>> obj = ExtSubTypeNoGC() + >>> obj = ExtSubTypeNoGC() + """ + + +@cython.test_fail_if_path_exists('//CClassDefNode[@scope.has_cyclic_pyobject_attrs = True]') +@cython.test_assert_path_exists('//CClassDefNode', + '//CClassDefNode[@scope]', + '//CClassDefNode[@scope.has_cyclic_pyobject_attrs = False]') +cdef class ExtTypePyArgsNoGC: + """ + >>> obj = ExtTypePyArgsNoGC() + >>> obj = ExtTypePyArgsNoGC() + >>> obj = ExtTypePyArgsNoGC() + >>> obj = ExtTypePyArgsNoGC() + >>> obj = ExtTypePyArgsNoGC() + >>> obj = ExtTypePyArgsNoGC() + """ + cdef bytes b + cdef str s + cdef unicode u + + +@cython.test_fail_if_path_exists('//CClassDefNode[@scope.has_cyclic_pyobject_attrs = True]') +@cython.test_assert_path_exists('//CClassDefNode', + '//CClassDefNode[@scope]', + '//CClassDefNode[@scope.has_cyclic_pyobject_attrs = False]') +cdef class ExtSubTypePyArgsNoGC(ExtTypePyArgsNoGC): + """ + >>> obj = ExtSubTypePyArgsNoGC() + >>> obj = ExtSubTypePyArgsNoGC() + >>> obj = ExtSubTypePyArgsNoGC() + >>> obj = ExtSubTypePyArgsNoGC() + >>> obj = ExtSubTypePyArgsNoGC() + >>> obj = ExtSubTypePyArgsNoGC() + """ + + +@cython.test_fail_if_path_exists('//CClassDefNode[@scope.has_cyclic_pyobject_attrs = False]') +@cython.test_assert_path_exists('//CClassDefNode', + '//CClassDefNode[@scope]', + '//CClassDefNode[@scope.has_cyclic_pyobject_attrs = True]') +cdef class ExtTypePyArgsWithGC: + """ + >>> obj = ExtTypePyArgsWithGC() + >>> obj = ExtTypePyArgsWithGC() + >>> obj = ExtTypePyArgsWithGC() + >>> obj = ExtTypePyArgsWithGC() + >>> obj = ExtTypePyArgsWithGC() + >>> obj = ExtTypePyArgsWithGC() + """ + cdef bytes b + cdef str s + cdef unicode u + cdef list l + + +@cython.test_fail_if_path_exists('//CClassDefNode[@scope.has_cyclic_pyobject_attrs = True]') +@cython.test_assert_path_exists('//CClassDefNode', + '//CClassDefNode[@scope]', + '//CClassDefNode[@scope.has_cyclic_pyobject_attrs = False]') +cdef class ExtSubTypePyArgsWithGC(ExtTypePyArgsWithGC): + """ + >>> obj = ExtSubTypePyArgsWithGC() + >>> obj = ExtSubTypePyArgsWithGC() + >>> obj = ExtSubTypePyArgsWithGC() + >>> obj = ExtSubTypePyArgsWithGC() + >>> obj = ExtSubTypePyArgsWithGC() + >>> obj = ExtSubTypePyArgsWithGC() + """ -- 2.7.4