classification
Title: Parser crash
Type: Stage:
Components: Interpreter Core Versions: Python 2.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: georg.brandl, loewis, nnorwitz
Priority: high Keywords:

Created on 2006-09-12 15:28 by loewis, last changed 2006-09-25 07:05 by georg.brandl. This issue is now closed.

Files
File name Uploaded Description Edit
ast-paren.diff nnorwitz, 2006-09-13 06:22 fix crash
ast-paren.diff nnorwitz, 2006-09-13 07:44 v2
tests.py nnorwitz, 2006-09-13 07:45 some simple tests
ast-paren.diff nnorwitz, 2006-09-18 05:55 v3
Messages (8)
msg29803 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2006-09-12 15:28
The code

def x(((y))):pass 

crashes the compiler (?) in 2.5c2, on Windows.
msg29804 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2006-09-13 05:50
Logged In: YES 
user_id=33168

The problem is in Python/ast.c around line 666.  See the
comment:

/* def foo((x)): setup for checking NAME below. */

The code is not sufficient, we need a loop and need to
handle various combinations of:

def f(((((x))),)),))): pass

I don't know if the parens above match, but the general idea
is that there could be a bunch of parens and commas at
various places.  I'm not sure how the above should be
interpreted.
msg29805 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2006-09-13 06:00
Logged In: YES 
user_id=33168

I guess what 2.4 does is the most reasonable behavior:

>>> def f((((((x)),),),)): pass
>>> dis.dis(f)
  1           0 LOAD_FAST                0 (.0)
              3 UNPACK_SEQUENCE          1
              6 UNPACK_SEQUENCE          1
              9 UNPACK_SEQUENCE          1
             12 STORE_FAST               1 (x)
             15 LOAD_CONST               0 (None)
             18 RETURN_VALUE        
msg29806 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2006-09-13 06:22
Logged In: YES 
user_id=33168

The attached patch seems to fix the ((((x)))) problem.  I
didn't run in debug mode, so I'm not positive the assert
works as I expect.

However now my test case below doesn't work, it puts in 5
UNPACK_SEQUENCES rather than 3.  This looks like a different
problem in compiler_complex_args().

Not sure how much farther I'll get tonight.
msg29807 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2006-09-13 07:44
Logged In: YES 
user_id=33168

I think patch v2 might fix both problems.  I'm not sure it's
correct.  We really need a lot of tests for this stuff.
msg29808 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2006-09-18 05:55
Logged In: YES 
user_id=33168

Attaching a new patch with tests.  There probably should be
more tests, but this is all I can think of at this point.  I
think I've covered the cases of the recursive definition
properly now.

Conceptually this is a very small patch.  I've added a bunch
of asserts and broken some complex lines up though.

The first chunk deals with complex_args and elides
superfluous parens around (x).  The second chunk does the
same eliding but for non-complex args.  Any number of extra
parens should be elided properly now.

I will apply to head and 2.5.1 later.
msg29809 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2006-09-22 08:21
Logged In: YES 
user_id=33168

Committed revision 51972. (2.6)
Still needs backport.
msg29810 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2006-09-25 07:05
Logged In: YES 
user_id=849994

Backported in rev. 51999.
History
Date User Action Args
2006-09-12 15:28:52loewiscreate