classification
Title: Bugs of the new AST compiler
Type: Stage:
Components: Interpreter Core Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: nnorwitz Nosy List: arigo, brett.cannon, mwh, nascheme, nnorwitz, rhettinger
Priority: critical Keywords:

Created on 2005-10-21 10:08 by arigo, last changed 2006-08-08 09:22 by arigo. This issue is now closed.

Files
File name Uploaded Description Edit
load-pop.diff nnorwitz, 2005-10-23 19:39 fix load const/pop top in compile.c
LOAD_CONST_POP_TOP.diff arigo, 2006-07-02 11:29
unary_minus_fix.txt nascheme, 2006-07-09 21:05
Messages (27)
msg26655 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2005-10-21 10:08
The newly merged AST branch is likely to expose
a number of small problems before it stabilizes,
so here is a tentative bug tracker entry to
collect such small problems.
msg26656 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2005-10-21 10:13
Logged In: YES 
user_id=4771

A scoping problem (comparing with the old compiler):

class X:
    print hello

The variable 'hello' is incorrectly looked up with
LOAD_GLOBAL instead of LOAD_NAME.  It causes a crash
in PyPy in a case where the name 'hello' is stored
into the class implicitely (via locals()).  It can
probably be discussed if the bug is in PyPy, but it
is a difference in behavior.
msg26657 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2005-10-21 10:22
Logged In: YES 
user_id=4771

import a as b, c

the 'c' part gets completely forgotten and there is
no 'IMPORT_NAME c' in the bytecode.
msg26658 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2005-10-21 11:54
Logged In: YES 
user_id=4771

any reason why lambda functions have a __name__ of
'lambda' now instead of '<lambda>' ?
msg26659 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2005-10-21 12:01
Logged In: YES 
user_id=4771

For reference, an optimization that got lost:

    def f():
        'a'
        'b'

'a' is the docstring, but the 'b' previously did not show
up anywhere in the code object.  Now there is the
LOAD_CONST/POP_TOP pair.
msg26660 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2005-10-21 12:15
Logged In: YES 
user_id=4771

The Python rules about which names get mangled are a bit
insane.  I share mwh's view that mangling should never have
been invented in the first place, but well:

>>> def f():
...   __x = 5
...   class X:
...     def g(self):
...       return __x
...   return X
... 
Fatal Python error: unknown scope for _X__x in X(135832776)
in <stdin>
msg26661 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2005-10-21 12:33
Logged In: YES 
user_id=4771

I would suspect the following one to be due to incorrect
handling of EXTENDED_ARG -- it's from a PyPy test about that:

longexpr = 'x = x or ' + '-x' * 2500
code = '''
def f(x):
    %s
    %s
    %s
    %s
    %s
    %s
    %s
    %s
    %s
    %s
    while x:
        x -= 1
        # EXTENDED_ARG/JUMP_ABSOLUTE here
    return x
''' % ((longexpr,)*10)

exec code
f(5)
SystemError: unknown opcode

dis.dis() shows that the target of both the SETUP_LOOP and
the JUMP_IF_FALSE at the start of the loop are wrong.
msg26662 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2005-10-21 12:45
Logged In: YES 
user_id=4771

The following (similarly strange-looking) code snippets
compiled successfully before, now they give SyntaxErrors:

--------------------
def f():
    class g:
        exec "hi"
        x
--------------------
def f(x):
    class g:
        exec "hi"
        x
--------------------
def f():
    class g:
        from a import *
        x
--------------------
def f(x):
    class g:
        from a import *
        x
msg26663 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2005-10-22 07:28
Logged In: YES 
user_id=33168

I assigned to Jeremy and Neil in the hopes they will see
this message and know about these problems.
msg26664 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2005-10-23 04:53
Logged In: YES 
user_id=80475

FWIW, here are the warnings issued by my compiler:

Python-ast.c
C:\py25\Python\Python-ast.c(1995) : warning C4101: 'i' : 
unreferenced local variable
C:\py25\Python\Python-ast.c(2070) : warning C4101: 'i' : 
unreferenced local variable
C:\py25\Python\Python-ast.c(2085) : warning C4101: 'i' : 
unreferenced local variable
C:\py25\Python\Python-ast.c(2130) : warning C4101: 'i' : 
unreferenced local variable
C:\py25\Python\Python-ast.c(2151) : warning C4101: 'i' : 
unreferenced local variable
C:\py25\Python\Python-ast.c(2261) : warning C4101: 'i' : 
unreferenced local variable
C:\py25\Python\Python-ast.c(2270) : warning C4101: 'i' : 
unreferenced local variable


compile.c
C:\py25\Python\compile.c(3782) : warning C4305: '=' : truncation 
from 'const int ' to 'char '
C:\py25\Python\compile.c(3802) : warning C4305: '=' : truncation 
from 'const int ' to 'char '
C:\py25\Python\compile.c(3806) : warning C4305: '=' : truncation 
from 'const int ' to 'char '
msg26665 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2005-10-23 19:14
Logged In: YES 
user_id=33168

Checkpoint of outstanding issues.  I think all the others
mentioned so far have been fixed:

 * Raymond's warnings in compile.c (unused locals are fixed)
 * EXTENDED_ARG problem
 * LOAD_CONST/POP_TOP (note we can fix this in the optimizer
generally which would get rid of other useless code too)
msg26666 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2005-12-04 10:30
Logged In: YES 
user_id=4771

At rev 41584, the following code snippet triggers an assert
if --with-pydebug is enabled:
Python/compile.c:3843: assemble_lnotab: Assertion 'd_lineno >= 0' failed

-----------------------
assert 1, ([s for s in x] +
           [s for s in x])
pass
-----------------------
msg26667 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2005-12-10 23:44
Logged In: YES 
user_id=357491

I just checked Armin's problem code on rev. 41638 and it
worked for me in the interpreter.  You still having
problems, Armin?
msg26668 - (view) Author: Michael Hudson (mwh) (Python committer) Date: 2005-12-11 00:41
Logged In: YES 
user_id=6656

You have to include those lines in a source file.  It still crashes for me.
msg26669 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2005-12-18 07:10
Logged In: YES 
user_id=33168

EXTENDED_ARG problem was fixed a while ago.
The assert/pass problem was fixed with: 41756.

That leaves the LOAD_CONST/POP_TOP optimization that was
lost and one compiler warning: marshal_write_mod() not being
used.
msg26670 - (view) Author: Michael Hudson (mwh) (Python committer) Date: 2006-02-09 15:02
Logged In: YES 
user_id=6656

We found another one.  Something is wrong in the compilation of augmented 
assignment to subscriptions containing tuples; running this code:

class C:
    def __setitem__(self, i, v):
        print i, v
    def __getitem__(self, i):
        print i
        return 0

c = C()
c[4,5] += 1

gives a spurious exception:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: object does not support item assignment

By contrast, "c[(4,5)] += 1" works fine.

msg26671 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2006-02-12 21:54
Logged In: YES 
user_id=4771

Subscripting is generally a bit sloppy: e.g. the AST model has
no way to distinguish between a single value and a one-element
tuple value!  See:

>>> d = {}
>>> d[1,] = 6
>>> d
{1: 6}        # !

I suggest we fix the model to turn the 'subs' of the 'Subscript' node
from a list of nodes to a single, mandatory 'sub' node.  If tupling is
necessary, it can be explicitly represented with a 'sub' containing a
'Tuple' node.
msg26672 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2006-02-18 06:56
Logged In: YES 
user_id=33168

Jeremy, there's no need to read anything before my last
comment at 2005-12-17 23:10.  The last two by Armin,
Michael, then my last comment are the only important ones. 
Everything that occurred before my 2005-12-17 comment was
taken care of AFAIK.
msg26673 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2006-04-03 07:30
Logged In: YES 
user_id=33168

The tuple store problem is fixed.  The only outstanding item
is the LOAD_CONST/POP_TOP.  I will fix that soon.
msg26674 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2006-04-11 08:45
Logged In: YES 
user_id=4771

Another one: the literal -2147483648 (i.e. the value of
-sys.maxint-1) gives a long in 2.5, but an int in <= 2.4.
msg26675 - (view) Author: Michael Hudson (mwh) (Python committer) Date: 2006-04-11 09:41
Logged In: YES 
user_id=6656

Good morning Armin!

I've reported that bug already: http://python.org/sf/1441486
There's a patch which purports to fix it: http://python.org/sf/1446922
but I haven't gotten around to testing it.

(this is running the pypy/module/array tests or something, isn't it?)
msg26676 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2006-07-02 11:29
Logged In: YES 
user_id=4771

Attached a patch for the LOAD_CONST POP_TOP optimization
(modified from Georg Brandl on python-dev).
msg26677 - (view) Author: Neil Schemenauer (nascheme) * (Python committer) Date: 2006-07-09 21:05
Logged In: YES 
user_id=35752

Regarding the -2147483648 bug: the old compiler tries to
fold unary +, - and ~ if the RHS is a constant.  I think
only the minus case is important since the others are just
optimizations, right?  Attaching what seems to be a minimum fix.
msg26678 - (view) Author: Neil Schemenauer (nascheme) * (Python committer) Date: 2006-07-09 21:23
Logged In: YES 
user_id=35752

Unary minus bug has been fixed in SVN rev 50495.
msg26679 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2006-08-03 07:26
Logged In: YES 
user_id=33168

Fix pending.  need tests and code to be thawed.
msg26680 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2006-08-04 05:10
Logged In: YES 
user_id=33168

I believe this addresses all the bugs.  If you find new
bugs, please open a new report so it's easier to track.

Committed revision 51081.
msg26681 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2006-08-08 09:22
Logged In: YES 
user_id=4771

Yes, I re-checked and all the bugs listed here are fixed.
History
Date User Action Args
2005-10-21 10:08:25arigocreate