Rovert, I've uploaded a new version of the patch following your
suggestions (though slightly different).

Please, see also my last comments on the tracker.

Unfortunately, I'll not be able to work further on this until Monday.
Good weekend!


On Thu, Apr 2, 2009 at 10:52 PM, Robert Bradshaw
<[email protected]> wrote:
> On Apr 2, 2009, at 4:39 PM, Lisandro Dalcin wrote:
>
>> - The changes to ModuleNode.py and Utils.py are just quick hacks to
>> get it working.
>>
>> - I've tried hard to avoid generation of unreachable code like these
>> "if (0) {...}" blocks.
>>
>> - Once this is reviewed, commented and eventually accepted, I'll
>> continue extending this to extern ctypedef integrals.
>
> I've taken a look at the code and it looks good to me. You took care
> of every corner case I thought of checking. I haven't tested it under
> Py3, but it works great under Py2. It might be worth noting that I
> improved the utility code templating to take a type since this is
> such a common case: http://hg.cython.org/cython-devel/file/
> 2289ab00261d/Cython/Utils.py
>
> One comment I had is that in your code
>
> static INLINE TYPE __Pyx_PyInt_AsTYPE(PyObject* x) {
>     if (sizeof(TYPE) < sizeof(long)) {
>         long val = __Pyx_PyInt_AsLong(x);
>         if (unlikely(val == -1 && PyErr_Occurred()))
>             return (TYPE)-1;
>         if (unlikely(val < 0)) {
>             PyErr_SetString(PyExc_OverflowError,
>                             "can't convert negative value to TYPE");
>             return (TYPE)-1;
>         }
>         if (unlikely(val != (long)(TYPE)val)) {
>             PyErr_SetString(PyExc_OverflowError,
>                             "value too large to convert to TYPE");
>             return (TYPE)-1;
>         }
>         return (TYPE)val;
>     }
>     return (TYPE)__Pyx_PyInt_AsUnsignedLong(x);
> }
>
> I believe the successful pathway could be shorted by doing
>
> static INLINE TYPE __Pyx_PyInt_AsTYPE(PyObject* x) {
>     if (sizeof(TYPE) < sizeof(long)) {
>         long val = __Pyx_PyInt_AsLong(x);
>         if (unlikely(val != (long)(TYPE)val)) {
>             if (unlikely(val == -1 && PyErr_Occurred()));
>             else if (val < 0) {
>                 PyErr_SetString(PyExc_OverflowError,
>                                 "can't convert negative value to
> TYPE");
>             }
>             else {
>                 PyErr_SetString(PyExc_OverflowError,
>                                 "value too large to convert to TYPE");
>             }
>             return (TYPE)-1;
>         }
>         return (TYPE)val;
>     }
>     return (TYPE)__Pyx_PyInt_AsUnsignedLong(x);
> }
>
> as sizeof(TYPE) < sizeof(long) guarantees val != (long)(TYPE)val for
> negative val on unsigned TYPE and the error does not need to be
> explicitly tested for otherwise (as the calling function will check
> PyErr_Occurred on a return value of -1).
>
> - Robert
>
> _______________________________________________
> Cython-dev mailing list
> [email protected]
> http://codespeak.net/mailman/listinfo/cython-dev
>



-- 
Lisandro Dalcín
---------------
Centro Internacional de Métodos Computacionales en Ingeniería (CIMEC)
Instituto de Desarrollo Tecnológico para la Industria Química (INTEC)
Consejo Nacional de Investigaciones Científicas y Técnicas (CONICET)
PTLC - Güemes 3450, (3000) Santa Fe, Argentina
Tel/Fax: +54-(0)342-451.1594
_______________________________________________
Cython-dev mailing list
[email protected]
http://codespeak.net/mailman/listinfo/cython-dev

Reply via email to