Thanks.

> string concatenation will now work, and errors like "'hello' - 'world'"
> should give a more informative TypeError
Yes, 'x'*5 works too.

> Bikeshed: We use Attribute rather than Attr for that node type,
> perhaps the full "Literal" name would be better
Lit seemed more in line with Num, Str, BinOp etc. No reason it can't be changed, of course.

> Lib/test/disutil.py should really be made a feature of the dis module
> itself
Agreed, but I didn't want to widen the scope of the patch. If this is something that can be reviewed quickly, I can make a change to dis. I'd add a keyword-only arg to dis and disassembly -- an output stream defaulting to stdout. dis_to_str then passes StringIO and returns the string. Sounds OK?

> Since the disassembly is interpreter specific, the new disassembly
> tests really shouldn't go directly in test_compile.py. A separate
> "test_ast_optimiser" file would be easier for alternate
> implementations to skip over.
New tests in test_compiler are not for the AST pass, but for changes to compile.c. I can split them out, how about test_compiler_opts?

> I'd like to see a written explanation for the first few changes in
> test_peepholer.py
Sure.
1) not x == 2 can be theoretically optimized to x != 2, while this test is for another optimization. not x is more robust.
2) Expression statement which is just a literal doesn't produce any code at all. This is now true for None/True/False as well. To preserve constants in the output I've put them in tuples.

> When you get around to rebasing the patch on 3.3 trunk, don't forget
> to drop any unneeded "from __future__" imports.
If you're referring to asdl_ct.py, that's actually an interesting question. asdl_ct.py is run by system installed python2, for obvious reasons. What is the policy here -- what is the minimum version of system python that should be sufficient to build python3? I tested my code on 2.6 and 3.1, and with __future__ it should work on 2.5 as well. Is this OK or should I drop 'with' so it runs on 2.4?

> The generated code for the Lit node type looks wrong: it sets v to
> Py_None, then immediately checks to see if v is NULL again.
Right, comment in asdl_c.py says:
# XXX: special hack for Lit. Lit value can be None and it
#  should be stored as Py_None, not as NULL.
If there's a general agreement on Lit I can certainly clean this up.

> Don't use "string" as a C type - use "char *" (and "char **" instead
> of "string *").
string is a typedef for PyObject that ASDL uses. I don't think I have a choice to not use it. Can you point to a specific place where char* could be used?

> There should be a new compiler flag to skip the AST optimisation step.
There's already an 'optimizations level' flag. Maybe we should make it more meaningful rather than multiplying the number of flags?

> A bunch of the compiler internal changes seem to make the basic flow
> of the generated assembly not match the incoming source code.
Can you give an example of what you mean?
The changes are basically 1) standard way of handling conditions in simple compilers 2) peephole.

> It doesn't seem like a good idea to conflate these with the AST
> optimisation patch. If that means leaving the peepholer in to handle
> them for now, that's OK - it's fine to just descope the peepholer
> without removing it entirely.
The reason why I think it makes sense to have this in a single change is testing. This allows to reuse all existing peephole tests. If I leave old peephole enabled there's no way to tell if my pass did something from disassembly. I can port tests to AST, but that seemed like more work than match old peepholer optimizations.
Is there any opposition to doing simple optimizations on compiler structures? They seem a good fit for the job. In fact, if not for stack representation, I'd say that they are better IR for optimizer than AST.

Also, can I get your opinion on making None/True/False into literals early on and getting rid of forbidden_name?

Antoine, Georg -- I think Nick's question is not about AST changing after optimizations (this can indeed be a separate flag), but the structure of AST changing. E.g. collapsing of Num/Str/Bytes into Lit.

Btw, if this is acceptable I'd make a couple more changes to make scope structure obvious from AST. This will allow auto-generating much of the symtable pass.