[Pyrex] Regression in 0.9.6.3 when raising an exception in __len__

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

John Arbash Meinel wrote:
> I have a custom __len__ implementation for one of my classes. Under certain
> conditions it can raise an exception. With Pyrex 0.9.6.3 my exception is
> getting silently ignored.
> 
> Looking at the generated .c code, I can see that 0.9.4.1 and 0.9.5.? have the
> error handlers looking like:
> 
>   __pyx_r = 0;
>   goto __pyx_L0;
>   __pyx_L1:;
>   __Pyx_AddTraceback("Class.__len__");
>   __pyx_r = -1;
>   __pyx_L0:;
>   Py_DECREF(__pyx_v_self);
>   return __pyx_r;
> 
> However, in 0.9.6.3 the "__pyx_r = -1;" line has been removed.
> 
> If I add it back manually, my tests pass again.
> 
> I'm guessing that __len__ returning -1 is how it indicates an exception case
> (like other functions return NULL to indicate an exception.)
> I'm thinking that probably the Py_ssize_t changes caused some warnings, so
> someone just removed that line, rather than doing something like:
> 
> __pyx_r = (Py_ssize_t) -1;
> 
> (Though -1 should be valid as a signed size_t so I don't know why it would be a
> warning.)
> 
> Is there a reason this was changed?
> 
> John
> =:->
> 

I did work out a patch which fixes this. It seems that it was the Py_ssize_t
code. Since it changes the Signature of the len() function.

This attached patch seems to fix it. I don't really like introducing a two
character signature, considering all others have a single character.

The other possibility would be a simpler diff:

- --- TypeSlots.py        2007-10-05 10:18:32.000000000 +0100
+++ TypeSlots.py        2007-11-16 20:17:32.000000000 +0000
@@ -56,6 +56,7 @@
         'i': "-1",
         'l': "-1",
         'r': "-1",
+        'Z': "-1",
     }

     def __init__(self, arg_format, ret_format):

I looked at other functions which return 'Z' and it was:
readbufferproc = Signature("TZP", "Z")
  # typedef Py_ssize_t (*readbufferproc)(PyObject *, Py_ssize_t, void **);

writebufferproc = Signature("TZP", "Z")
  # typedef Py_ssize_t (*writebufferproc)(PyObject *, Py_ssize_t, void **);

segcountproc = Signature("TZ", "Z")
  # typedef Py_ssize_t (*segcountproc)(PyObject *, Py_ssize_t *);

writebufferproc = Signature("TZS", "Z")
  # typedef Py_ssize_t (*charbufferproc)(PyObject *, Py_ssize_t, char **);

and while I don't know those functions very well, they didn't seem like things
that returning -1 would be valid.

For now, I'm going with the attached one, because it has a smaller (more
conservative) impact.

Oh, and I tried to download the hg tarball of Pyrex so I could work with it
instead of raw directories, but it seems to contain a lot of ".Foo.py" files.
I'm pretty sure this is because whatever tar program was used was "Mac OS X"
aware and was tarring up the resource forks. It would be nice if those weren't
in the normal tarball.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHPfwfJdeBCYSNAAMRAj4MAJ4o4ifyJCMbmcDmZgM9GkCyfayEnwCeIxjn
N3m5rTEklBmyq9hxed+s8PU=
=QEnT
-----END PGP SIGNATURE-----
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: len_exception.diff
Url: http://lists.copyleft.no/pipermail/pyrex/attachments/20071116/cb513ace/attachment.ksh