Skip to content

Commit

Permalink
Allow cdef class to inherit from PyVarObject if it does not change C …
Browse files Browse the repository at this point in the history
…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 cython#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: cython#711
  • Loading branch information
navytux committed Jan 17, 2023
1 parent c21b39d commit 4671623
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 11 deletions.
2 changes: 2 additions & 0 deletions Cython/Compiler/Builtin.py
Expand Up @@ -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)
Expand Down
20 changes: 16 additions & 4 deletions Cython/Compiler/Nodes.py
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
26 changes: 19 additions & 7 deletions 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
"""
33 changes: 33 additions & 0 deletions tests/run/builtin_type_inheritance_T608.pyx
Expand Up @@ -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) + ")"

0 comments on commit 4671623

Please sign in to comment.