| msg25072 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2005-04-19 19:02 |
| test_genexps is failing because it is unable to find a global defined in a genexp that is returned. Here is the problem simplified: def f(n): return (i for i in xrange(n)) list(f(10)) Leads to ``SystemError: no locals when loading 'xrange'``. Comparing Python 2.4 bytecode: 1 0 LOAD_CONST 1 (<code object at 0x3931e0, file "", line 1>) 3 MAKE_FUNCTION 0 6 LOAD_GLOBAL 0 (xrange) 9 LOAD_FAST 0 (n) 12 CALL_FUNCTION 1 15 GET_ITER 16 CALL_FUNCTION 1 19 RETURN_VALUE 20 LOAD_CONST 0 (None) 23 RETURN_VALUE to AST bytecode: 1 0 LOAD_CLOSURE 0 (n) 3 BUILD_TUPLE 1 6 LOAD_CONST 1 (<code object at 0x5212e8, file "", line 1>) 9 MAKE_CLOSURE 0 12 LOAD_NAME 0 (xrange) 15 LOAD_DEREF 0 (n) 18 CALL_FUNCTION 1 21 GET_ITER 22 CALL_FUNCTION 1 25 RETURN_VALUE 26 LOAD_CONST 0 (None) 29 RETURN_VALUE makes it obvious something is off (no peepholer; turned it off in my build of 2.4). Looks like extraneous work is being done in making a closure. Seems like it will still work, though. Plus the usage of LOAD_NAME is wrong in the AST; LOAD_NAME gets an object from the local namespace based on its name instead of offset. |
|
|
| msg25073 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2005-04-19 19:03 |
| Logged In: YES user_id=357491 Initially assigned to Nick since he did the genexp patch and in hopes he might know what is going on off the top of his head. Otherwise assign to me. I have a sneaking suspicion that the symtable code overall is slightly busted. |
|
|
| msg25074 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2005-04-20 00:09 |
| Logged In: YES user_id=357491 Some playing with gdb has turned up some clues. So LOAD_NAME is emitted by compiler_nameop(). How this partially works is that it gets the scope for an argument and then based on that emits the proper load, store, or delete. So why is xrange() coming out at the NAME scope? Well, turns out it is not being found in a particular scope by PyST_GetScope() and is thus returning 0 by default. This means that NAME becomes the scope for xrange(). So it looks like the symtable creation is screwing up and not making xrange() a global like it should. This might be a side-effect of the while closure thing above. Not sure, though. |
|
|
| msg25075 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2005-04-20 00:30 |
| Logged In: YES user_id=357491 OK, figured out why the closure thing is happening. 'n' is being detected as a free variable. This leads to PyCode_GetNumFree() to return a non-0 value. In compiler_make_closure() this automatically triggers the LOAD_CLOSURE/.../MAKE_CLOSURE chunk of bytecode instead of LOAD_CONST/MAKE_FUNCTION. So, how to make 'n' be detected as a local instead a free variable ... |
|
|
| msg25076 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2005-04-20 22:13 |
| Logged In: YES user_id=1038590 The compiler stage does some fancy footwork so that the outermost iterable is evaluated immediately, and passed as an argument to the genexp block. The symbol table generation needs similar footwork, but it doesn't have it. That is, the symbol table for the outermost iterable is being created in the genexp scope, instead of in the containing function's scope, but the actual execution is happening in the function scope. Hilarity ensues. . . My original testing didn't pick this up because I was using (1, 2, 3) as the outermost iterable - no name lookups in the outermost iterable to be affected! If you haven't figured out a fix for this part of the problem before Sunday, I should be able to get to it then. The location of the enter_scope and exit_scope in the symbol table pass for genexps needs adjusting so that the outermost iterable is NOT part of the new scope. |
|
|
| msg25077 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2005-04-21 06:13 |
| Logged In: YES user_id=357491 OK, so basically what I figured out earlier today. I noticed that both genexps in 2.4 and the AST branch generated the same bytecode, so the only difference was how it was handling passing in arguments to the generator for its initial creation. I figured there was some jiggering in the compiler to note that the scoping in the genexp should not affect how variables were handled for its arguments. And as you said, the AST branch just doesn't take this into consideration as it should. I will see if I get to it, but I doubt I will (leaving town Friday for the weekend). I think this has justified fleshing out Python/compile.txt with a more thorough discussion of how the symbol table is generated. If you have any specific points you think should go in there, Nick (or anyone for that matter), please email them to me. Even explaining how the whole thing works would be a help. =) |
|
|
| msg25078 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2005-05-28 07:32 |
| Logged In: YES user_id=1038590 Patch attached that means test_genexps mostly passes for me. The two which still fail are actually problems with assignment statements (the AST branch does not yet intercept attempts to assign to genexps and produce a nice error message). I'm also much happier with the implementation technique I used this time around - something similar should work for the compiler implementation, hopefully permitting it to be simplified (I'm going to look at that now, and will post a separate patch if it works out). |
|
|
| msg25079 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2005-05-28 07:33 |
| Logged In: YES user_id=1038590 Tag, you're it :) |
|
|
| msg25080 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2005-05-28 07:49 |
| Logged In: YES user_id=1038590 Scratch that comment about applying a similar methodology to the compiler stage - the need to correctly structure the generate bytecode makes things a lot uglier, so the simple trick I used to get the symbol table scoping right doesn't apply (the outer loops need to do things before *and after* the inner loops in the compiler stage). |
|
|
| msg25081 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2005-07-11 03:53 |
| Logged In: YES user_id=357491 rev. 2.10.8.34 has the patch. Thanks, Nick. |
|
|