From 4671623c71c6a8641e565e72cf88d6be2b5c39ab Mon Sep 17 00:00:00 2001 From: Kirill Smelkov Date: Tue, 17 Jan 2023 13:49:27 +0300 Subject: [PATCH] Allow cdef class to inherit from PyVarObject if it does not change C struct layout in other words allow subtyping PyVarObject (e.g. bytes/tuple) without adding C-level fields or otherwise changing C-level struct layout at all. At https://github.com/cython/cython/issues/711 @robertwb says: The problem with a PyVarObject (such as str) is that its struct is of variable length that is determined at instance creation time. When Cython generates subclass code, it expects to be able to add fields directly behind the compile time struct, which thus end up in the variably allocated memory area So the problem with inheriting from PyVarObject is that accessing C-level attributes needs to be adjusted to know the real runtime size of the object. However for simple cases, like e.g. cdef class MyBytes(bytes): # no cdef attributes def meth1() def meth2() ... we know it can already work ok as is because if there is no C-level attributes, then there is no problem of accessing them. In general if we do not change the C-structure layout of the object compared to its base type, it can work already with existing infrastructure without any change. Such inheritance can be useful even in the limited form. For example in https://lab.nexedi.com/nexedi/pygolang/merge_requests/21 I use it to inherit from `bytes` with making sure that there is no size increase in inherited object. It already works ok out of the box for `unicode`, but previously even cdef class MyBytes(bytes): pass was rejected. Updates: https://github.com/cython/cython/issues/711 --- Cython/Compiler/Builtin.py | 2 ++ Cython/Compiler/Nodes.py | 20 ++++++++++--- tests/errors/builtin_type_inheritance.pyx | 26 +++++++++++----- tests/run/builtin_type_inheritance_T608.pyx | 33 +++++++++++++++++++++ 4 files changed, 70 insertions(+), 11 deletions(-) diff --git a/Cython/Compiler/Builtin.py b/Cython/Compiler/Builtin.py index e0d203ae027..0508cfbace5 100644 --- a/Cython/Compiler/Builtin.py +++ b/Cython/Compiler/Builtin.py @@ -395,6 +395,8 @@ def init_builtin_types(): objstruct_cname = "PyBaseExceptionObject" elif name == 'StopAsyncIteration': objstruct_cname = "PyBaseExceptionObject" + elif name == 'str': + objstruct_cname = "PyStringObject" else: objstruct_cname = 'Py%sObject' % name.capitalize() the_type = builtin_scope.declare_builtin_type(name, cname, utility, objstruct_cname) diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py index c5ddad0a6c3..54461c879ce 100644 --- a/Cython/Compiler/Nodes.py +++ b/Cython/Compiler/Nodes.py @@ -4756,10 +4756,7 @@ def analyse_declarations(self, env): base_type.is_final_type: error(base.pos, "Base class '%s' of type '%s' is final" % ( base_type, self.class_name)) - elif base_type.is_builtin_type and \ - base_type.name in ('tuple', 'str', 'bytes'): - error(base.pos, "inheritance from PyVarObject types like '%s' is not currently supported" - % base_type.name) + # NOTE base being PyVarObject builtin type is checked in the end else: self.base_type = base_type if env.directives.get('freelist', 0) > 0 and base_type != PyrexTypes.py_object_type: @@ -4851,6 +4848,21 @@ def analyse_declarations(self, env): for thunk in self.entry.type.defered_declarations: thunk() + # reject cdef class that inherits from PyVarObject builtin and changes struct size + # do the verification in the end when we know whether vtable needed to be instantiated + # https://github.com/cython/cython/issues/711 + if self.base_type and \ + self.base_type.is_builtin_type and \ + self.base_type.name in ('tuple', 'str', 'bytes'): + if self.entry.type.scope.var_entries: + error(base.pos, ("inheritance from PyVarObject types '%s' " + "with C-level attributes is not currently supported") + % self.base_type.name) + if self.entry.type.vtabslot_cname: + error(base.pos, ("inheritance from PyVarObject types '%s' " + "with C-level vtable is not currently supported") + % self.base_type.name) + def analyse_expressions(self, env): if self.body: scope = self.entry.type.scope diff --git a/tests/errors/builtin_type_inheritance.pyx b/tests/errors/builtin_type_inheritance.pyx index 1c6ad31e17b..01762586285 100644 --- a/tests/errors/builtin_type_inheritance.pyx +++ b/tests/errors/builtin_type_inheritance.pyx @@ -1,18 +1,30 @@ # mode: error -# current restriction: cannot inherit from PyVarObject (see ticket #152) +# current restriction: cannot inherit from PyVarObject if C structure changes (see ticket #152) cdef class MyTuple(tuple): - pass + cdef object attr cdef class MyBytes(bytes): - pass + cdef object attr cdef class MyStr(str): # only in Py2, but can't know that during compilation - pass + cdef object attr + +cdef class MyTuple2(tuple): + cdef meth(self): pass + +cdef class MyBytes2(bytes): + cdef meth(self): pass + +cdef class MyStr2(str): # only in Py2, but can't know that during compilation + cdef meth(self): pass _ERRORS = """ -5:19: inheritance from PyVarObject types like 'tuple' is not currently supported -8:19: inheritance from PyVarObject types like 'bytes' is not currently supported -11:17: inheritance from PyVarObject types like 'str' is not currently supported +5:19: inheritance from PyVarObject types 'tuple' with C-level attributes is not currently supported +8:19: inheritance from PyVarObject types 'bytes' with C-level attributes is not currently supported +11:17: inheritance from PyVarObject types 'str' with C-level attributes is not currently supported +14:20: inheritance from PyVarObject types 'tuple' with C-level vtable is not currently supported +17:20: inheritance from PyVarObject types 'bytes' with C-level vtable is not currently supported +20:18: inheritance from PyVarObject types 'str' with C-level vtable is not currently supported """ diff --git a/tests/run/builtin_type_inheritance_T608.pyx b/tests/run/builtin_type_inheritance_T608.pyx index 67e97ec1e58..57bf4d79b2d 100644 --- a/tests/run/builtin_type_inheritance_T608.pyx +++ b/tests/run/builtin_type_inheritance_T608.pyx @@ -148,3 +148,36 @@ def test_exception_type_cast(Exception maybe_exn): """ cdef object o = maybe_exn cdef Exception e = o + + +# inheritance from PyVarObject with no change to C-level structure + +cdef class MyTuple(tuple): + """ + >>> MyTuple([1,2,3]) == (1,2,3) + True + >>> repr(MyTuple([1,2,3])) + 'MyTuple(1, 2, 3)' + """ + def __repr__(self): + return "MyTuple" + tuple.__repr__(self) + +cdef class MyBytes(bytes): + """ + >>> MyBytes(b'abc') == b'abc' + True + >>> repr(MyBytes(b'zzz')) + 'b(MyBytes)' + """ + def __repr__(self): + return "b(MyBytes)" + +cdef class MyStr(str): # PyVarObject only in Py2 + """ + >>> MyStr('abc') == 'abc' + True + >>> repr(MyStr('abc')) + "MyStr('abc')" + """ + def __repr__(self): + return "MyStr(" + str.__repr__(self) + ")"