[Cython] cdef class tp_new should call PyBaseObject_Type.tp_new?
Hi all,
I recently posted to the cython-users list about this problem. This
email is to submita potential patch to fix the issue. The issue is as
follows:
At Enthought we are busy porting Traits
(http://github.com/enthought/traits) from C to Cython.
An issue has come up that that prevents classes inheriting from a
cdef-class from playing nicely with ABCs. It is type's tp_new that
checks for abstractmethods that have not been implemented, so if type's
tp_new is bypassed, the check is never run and you can instantiate an
abstract base class.
It is equivalent to this simple example below (taken from
http://stackoverflow.com/questions/20432335). If you replace the simple
class A with a Cython cdef class, the effect is the same.
import abc
class A(object):
def __new__(cls):
# self = object.__new__(cls)
return 42
class B(A):
__metaclass__ = abc.ABCMeta
@abc.abstractmethod
def foo(self):
pass
b = B() # No exception.
Thanks,
Simon
From 80d518908ad3f2052e4d9e7fcf8101fc5d75280e Mon Sep 17 00:00:00 2001
From: Simon Jagoe
Date: Wed, 8 Jan 2014 20:00:48 +
Subject: [PATCH] BUG: Call PyBaseObject_Type.tp_new instead of
NewType->tp_alloc
---
Cython/Compiler/ModuleNode.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Cython/Compiler/ModuleNode.py b/Cython/Compiler/ModuleNode.py
index 338efc0..c2795fe 100644
--- a/Cython/Compiler/ModuleNode.py
+++ b/Cython/Compiler/ModuleNode.py
@@ -1134,7 +1134,7 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode):
if scope.needs_gc():
code.putln("PyObject_GC_Track(o);")
code.putln("} else {")
-code.putln("o = (*t->tp_alloc)(t, 0);")
+code.putln("o = (PyObject *) PyBaseObject_Type.tp_new(t, __pyx_empty_tuple, 0);")
code.putln("if (unlikely(!o)) return 0;")
if freelist_size and not base_type:
code.putln('}')
--
1.8.3.2
___
cython-devel mailing list
cython-devel@python.org
https://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] cdef class tp_new should call PyBaseObject_Type.tp_new?
Hi Stefan, > Looking at the code in Py3, ISTM that it would be sufficient to do this iff > > (t->tp_flags & Py_TPFLAGS_IS_ABSTRACT) != 0 > > which should be rare enough. > > Obviously requires some user side testing to see if it breaks any other > code... Thanks for looking at this. I am wondering if there is a reason why the tp_alloc can't be replaced with the PyBaseObject_Type->tp_new directly? My understanding is that initially, object's tp_new essentially just called tp_alloc as Cython does now. Later the abc feature was added with the abstract check happening in object's tp_new, while not changing previous (non-abstract class) behavior. If Cython were to use this instead of tp_alloc, it would be forward-compatible with future changes to object. Aside from your comment that it would need extensive testing, am I missing some other problems? Thanks, Simon ___ cython-devel mailing list cython-devel@python.org https://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] cdef class tp_new should call PyBaseObject_Type.tp_new?
Thanks, Stefan. Let me know if I can help in any way. I will track Cython master while I continue my port from C to Cython. On 10 January 2014 09:52, Stefan Behnel wrote: > Stefan Behnel, 10.01.2014 10:11: >> Simon Jagoe, 09.01.2014 14:04: >>>> Looking at the code in Py3, ISTM that it would be sufficient to do this iff >>>> >>>> (t->tp_flags & Py_TPFLAGS_IS_ABSTRACT) != 0 >>>> >>>> which should be rare enough. >>>> >>>> Obviously requires some user side testing to see if it breaks any other >>>> code... >>> >>> Thanks for looking at this. I am wondering if there is a reason why the >>> tp_alloc can't be replaced with the PyBaseObject_Type->tp_new directly? >> >> I don't know. At least, it's some unnecessary overhead in almost all cases. >> Not sure if that matters, there's enough costly stuff happening at this >> point anyway. >> >> ISTM that we'd also have to fix the issue that Nils Bruin found, otherwise >> we'd risk even more trouble on user side. >> >> http://thread.gmane.org/gmane.comp.python.cython.user/10267 >> >> >>> My understanding is that initially, object's tp_new essentially just >>> called tp_alloc as Cython does now. Later the abc feature was added >>> with the abstract check happening in object's tp_new, while not changing >>> previous (non-abstract class) behavior. If Cython were to use this >>> instead of tp_alloc, it would be forward-compatible with future changes >>> to object. >> >> Agreed. > > Let's see what Jenkins has to say about it. > > https://github.com/cython/cython/commit/ddaffbe78c06e580ca0d9cd334c7a28a4de5c40b > > Stefan > > ___ > cython-devel mailing list > cython-devel@python.org > https://mail.python.org/mailman/listinfo/cython-devel ___ cython-devel mailing list cython-devel@python.org https://mail.python.org/mailman/listinfo/cython-devel
