[Cython] ready to push changes to make Cython understand 'size_t' as a native C type

[Stefan Behnel]

Lisandro Dalcin wrote:
>>> The only thing that I'm not completely sure about is the
>>> implementation of this, though it works fine in my 32bit Linux box
>>>
>>> static INLINE size_t __pyx_PyInt_AsSize_t(PyObject* b);
> 
> http://trac.cython.org/cython_trac/attachment/ticket/207/SIZE_T.diff#preview

	static INLINE size_t __pyx_PyInt_AsSize_t(PyObject* b) {
 		 unsigned PY_LONG_LONG val =
				 __pyx_PyInt_AsUnsignedLongLong(b);
 		 if (unlikely(val == (unsigned PY_LONG_LONG)-1 &&
				 PyErr_Occurred())) {
 		       return (size_t)-1;

The "(size_t)-1" definitely looks funny. I noticed that "(unsigned
PY_LONG_LONG)-1" is also used in __pyx_PyInt_AsUnsignedLongLong(), so I
just hope it does the right thing.


 		  } else if (unlikely((unsigned PY_LONG_LONG)(size_t)val
							 != val)) {
 		       PyErr_SetString(PyExc_OverflowError, "value too
					 large to convert to size_t");

This should return an error value, so it lacks a "return (size_t)-1".

 		  }
 		  return val;
	}

It would be great if we had tests for the error cases. It's not obvious
that they work as expected. The same applies to the "unsigned long long"
conversion, IMHO.


>>> Of course, this change is not backward compatible... Cython will not
>>> compile the code:
>>>
>>> cdef extern from "stdlib.h":
>>>     ctypedef unsigned long size_t
>> Is this because of an explicit error, or just a side effect of the
>> implementation?
>>
> 
> Cython generates the following error (IMHO, that's a good thing)
> 
> Error converting Pyrex file to C:
> ------------------------------------------------------------
> ...
> #cython: embedsignature=True
> ctypedef unsigned long size_t
>                             ^
> ------------------------------------------------------------
> 
> /u/dalcinl/Devel/mpi4py-dev/src/mpi4py.MPI.pyx:2:29: Empty declarator

Not really the most telling error message, but not cryptic either. Fine
with me.


>>> What should be the behavior if the user tries to ctypedef 'Py_ssize_t'
>>> or 'size_t', provided that Cython treats them as pre-defined C types?
>> I didn't care at the time when I implemented the Py_ssize_t stuff, but when
>> you ask now, I prefer getting at least a warning. I doubt that Cython
>> handles a "ctypedef ... int" gracefully, for example.
> 
> Sorry for the noise, I got confused.. Cython DO FAIL if you 'ctypedef
> ... Py_ssize_t'.

Same as above, I assume: it thinks that you are still collecting types and
modifiers, so it's missing the new type name at the end.


>>> Finally, can I push the changes?
>> Could you file a bug report for now (if only for documentation purposes)
>> and post the patch there? We should be a bit careful with major changes by
>> now, as we are currently trying to stabilise Cython for the 0.11 release.
>>
> 
> OK. Just hope this can get in for the next release.

Apart from what I said above, it looks ok, so I'd like to see it in as well.

Stefan