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
_______________________________________________
Cython-dev mailing list
[email protected]
http://codespeak.net/mailman/listinfo/cython-dev
