[Cython] Some bugs found while testing cython on django

[Stefan Behnel]

Robert Bradshaw, 21.06.2011 01:14:
> On Mon, Jun 20, 2011 at 12:14 AM, Stefan Behnel wrote:
>> Vitja Makarov, 19.06.2011 21:37:
>>>
>>> Trying to compile django I've found some problems:
>>>
>>> 1. It seems that cython currently doesn't support tuples inside args
>>> definition:
>>>
>>> def foo((a, b), c):
>>>
>>> Currently this gives missing argument name and crash inside
>>> CreateControlFlow graph
>>>
>>> cython -X binding=True django/contrib/gis/forms/fields.py -o
>>> django/contrib/gis/forms/fields.c
>>
>> It may be valid Py2-only syntax, but I consider its usage a bug in Django.
>
> I don't know about the parser side of things

Sadly, that's the bigger part of the problem. The parser code for function 
arguments is pretty complex because it basically has to parse C syntax at 
this point. I tried to implement the parser part before I opened ticket 692 
and it turned out to be a major ugliness. Currently, the parser even reads 
this without complaining. I don't recall the exact details, but I think it 
was  because it resembles a part of a C function type declaration or something.

Once we get this through the parser correctly, it won't be all that hard to 
implement anymore. However, IMHO, it's a lot of work overall for a dead 
feature.


>>> 2. Concatenating unicode and str isn't supported:
>>>
>>> I guess that this is mostly django problem
>>>
>>> Error compiling Cython file:
>>> ------------------------------------------------------------
>>> ...
>>>      default_error_messages = {
>>>          'no_geom' : _(u'No geometry value provided.'),
>>>          'invalid_geom' : _(u'Invalid geometry value.'),
>>>          'invalid_geom_type' : _(u'Invalid geometry type.'),
>>>          'transform_error' : _(u'An error occurred when transforming
>>> the geometry '
>>>                                'to the SRID of the geometry form field.'),
>>>                               ^
>>> ------------------------------------------------------------
>>>
>>> django/contrib/gis/forms/fields.py:21:30: Cannot mix string literals
>>> of different types, expected u'', got ''
>>
>> That's a bug in Django. Worth submitting a patch to their bug tracker. It
>> works in this case, but it's not what they want in their code:
>>
>>     >>>  u'abc\u1234'
>>     u'abc\u1234'
>>     >>>  u'abc' '\u1234'
>>     u'abc\\u1234'
>
> This error is only for literals (or otherwise typed value, right?)

This is *only* about string literals and their auto-concatenation by the 
parser.


> I might say it's not a bug in their code--the second unicode marker is
> redundant here if you know how things work in Py2.

Right, *if* you know it. So the above will fail to provide the correct 
result if someone who happens to be not aware of these details changes the 
literal to something that contains a non-ASCII character. And this problem 
may or may not show up in a test, as the above is only a text message, even 
just an (unlikely?) error message.

Rather error prone, if you ask me. Which lets me suspect that the code was 
written by someone who was not aware of the details in the first place.

I must say, I like the error message that Cython outputs here. Would have 
caught that problem earlier.  :)


>>> 3.Reraise not inside except clause, here is simple example
>>>
>>> def foo():
>>>      print 'catched'
>>>      raise
>>> try:
>>>      raise IndexError
>>> except IndexError:
>>>      foo()
>>
>> It would be possible to support this, given that we are compatible with
>> CPython regarding the place where exceptions are stored while being handled.
>> Despite of what I initially thought, this is a feature of both Py2 and Py3,
>> even though I expect it to be rarely used and somewhat error prone.
>
> It's a bit odd, but should still be supported.

Oh, absolutely. I should have stated that clearer.

Stefan