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

[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