[Cython] [PATCH] Re: view[0].methodcall() produces invalid C when view is memoryview of extension type

2013-04-15 Thread Matěj Laitl
Hi again, especially Mark,
I have some new observations and a patch regarding this problem, see below.

On 23. 3. 2013 Matěj Laitl wrote:
> following test code produces C code that fails to compile:
> > cdef class ExtensionType(object):
> > cdef public int dummy
> > 
> > def __init__(self, n):
> > self.dummy = n
> > 
> > cdef cfoo(self):
> > print self.dummy
> > 
> > items = [ExtensionType(1), ExtensionType(2)]
> > cdef ExtensionType[:] view = np.array(items, dtype=ExtensionType)
> > view[0].cfoo()
> 
> with gcc error and relevant C file lines:
> extension_type_memoryview.c:2604:94: error: ‘PyObject’ has no member named
> ‘__pyx_vtab’
> 
> 2570:  PyObject *__pyx_t_1 = NULL;
> (...)
> 2601:  __pyx_t_1 = (PyObject *) *((struct
> 
> :__pyx_obj_25extension_type_memoryview_ExtensionType * *) ( /* dim=0 */
> :
> : (__pyx_v_25extension_type_memoryview_view.data + __pyx_t_2 *
> : __pyx_v_25extension_type_memoryview_view.strides[0]) ));
> 
> 2602:  __Pyx_INCREF((PyObject*)__pyx_t_1);
> 2603:  /* __pyx_t_4 allocated */
> 2604:  __pyx_t_4 = ((struct
> 
> : __pyx_vtabstruct_25extension_type_memoryview_ExtensionType *)__pyx_t_1
> :
> :->__pyx_vtab)->cfoo(__pyx_t_1); if (unlikely(!__pyx_t_4))
> :{__pyx_filename
> :
> : = __pyx_f[0]; __pyx_lineno = 69;
> 
> It seems that generic PyObject* temporary for __pyx_t_1 is used here while
> typed ExtensionType* temporary should have been used instead (as suggested
> by excess casting on line 2601).

I've found this is caused by following check in 
ExprNode.allocate_temp_result():
> if type.is_pyobject:
> type = PyrexTypes.py_object_type

Removing the check breaks everything else, so I suppose it is intentional 
(what's its purpose?) and I've resorted to overriding allocate_temp_result() 
in IndexNode and skipping the check in the very special case of memoryview 
direct indexing.

This fixes the test-case and doesn't break other tests, but sounds a bit hacky 
and may be actually completely incorrect.

The patch and test-case is here: https://github.com/cython/cython/pull/213

P.S.: Is this too late for 0.19?

Cheers,
Matěj
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] [PATCH] Re: view[0].methodcall() produces invalid C when view is memoryview of extension type

2013-04-15 Thread Stefan Behnel
Matěj Laitl, 15.04.2013 17:47:
> P.S.: Is this too late for 0.19?

Unless one of the other core developers signs that this is totally correct
and safe and must be merged, I'd say yes, too late, sorry.

Stefan

___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel