[issue11549] Rewrite peephole to work on AST

Nick Coghlan added the comment: Eugene raised the question of AST changes on python-dev [1] and the verdict was that so long as ast.__version__ is updated, AST clients will be able to cope with changes. Benjamin Peterson made some subsequent changes to the AST (bringing the AST for try and w

[issue11549] Rewrite peephole to work on AST

Nick Coghlan added the comment: Eugene: I suggest raising the question on python-dev. The answer potentially affects the PEP 380 patch as well (which adds a new attribute to the "Yield" node). Anatoly: If you just want to get a feel for the kind of AST various pieces of code produce, then th

[issue11549] Rewrite peephole to work on AST

anatoly techtonik added the comment: Is there any tool to see how it works step-by-step. The whole stuff is extremely interesting, but I can't fit all the details of AST processing in my head. -- nosy: +techtonik ___ Python tracker

[issue11549] Rewrite peephole to work on AST

Raymond Hettinger added the comment: Eugene, I think you're doing great work here and would like to see you succeed. In the near term, I don't have time to participate, but don't let that stop you. -- ___ Python tracker

[issue11549] Rewrite peephole to work on AST

Eugene Toder added the comment: If we have to preserve backward compatibility of Python AST API, we can do this relatively easily (at the expense of some code complexity): * Add 'version' argument to compile() and ast.parse() with default value of 1 (old AST). Value 2 will correspond to the ne

[issue11549] Rewrite peephole to work on AST

Nick Coghlan added the comment: OK, I missed the fact that the new optimisation pass isn't run under PyCF_ONLY_AST. However, as Eugene picked up, my concern is actually with the collapsing of Str/Num/Bytes/Ellipsis into the new Lit node, and the changes to the way None/True/False are handled

[issue11549] Rewrite peephole to work on AST

Terry J. Reedy added the comment: While I would not be happy to use class X above, the 3.2 manual explicitly says "There are no implied relationships among the comparison operators. The truth of x==y does not imply that x!=y is false. " . -- ___ Py

[issue11549] Rewrite peephole to work on AST

Eugene Toder added the comment: Also, to avoid any confusion -- currently my patch only runs AST optimizations before code generation, so compile() with ast.PyCF_ONLY_AST returns non-optimized AST. -- ___ Python tracker

[issue11549] Rewrite peephole to work on AST

Eugene Toder added the comment: > I don't think it can: That already doesn't work in dict and set (eq not consistent with hash), I don't think it's a big problem if that stops working in some other cases. Anyway, I said "theoretically" -- maybe after some conservative type inference.

[issue11549] Rewrite peephole to work on AST

Daniel Urban added the comment: > not x == 2 can be theoretically optimized to x != 2, ... I don't think it can: >>> class X: ... def __eq__(self, other): ... return True ... def __ne__(self, other): ... return True ... >>> x = X() >>> >>> not x == 2 False >>>

[issue11549] Rewrite peephole to work on AST

Eugene Toder added the comment: > and with __future__ it should work on 2.5 as well. Actually, seems that at least str.format is not in 2.5 as well. Still the question is should I make it run on 2.5 or 2.4 or is 2.6 OK (then __future__ can be removed). --

[issue11549] Rewrite peephole to work on AST

Eugene Toder added the comment: 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

[issue11549] Rewrite peephole to work on AST

Georg Brandl added the comment: I would provide this via another compile flag a la PyCF_ONLY_AST. If you give only this flag, you get the original AST. If you give (e.g.) PyCF_OPTIMIZED_AST, you get the resulting AST after the optimization stage (or the same, if optimization has been disable

[issue11549] Rewrite peephole to work on AST

Antoine Pitrou added the comment: > As a more general policy question... where do we stand in regards to > backwards compatibility of the AST? The ast module docs don't have any > caveats to say that it may change between versions, but it obviously > *can* change due to new language constructs (

[issue11549] Rewrite peephole to work on AST

Nick Coghlan added the comment: I think the biggest thing to take out of my review is that I strongly encourage deferring the changes for 5(b) and 5(c). I like the basic idea of using a template-based approach to try to get rid of a lot of the boilerplate code currently needed for AST visitor

[issue11549] Rewrite peephole to work on AST

Nick Coghlan added the comment: Finally got around to reviewing this (just a visual scan at this stage) - thanks for the effort. These are mostly "big picture" type comments, so I'm keeping them here rather than burying them amongst all the details in the code review tool. The effect that co

[issue11549] Rewrite peephole to work on AST

Terry J. Reedy added the comment: I suspect someone will sometime. There is bit of a backlog of older issues. -- ___ Python tracker ___ _

[issue11549] Rewrite peephole to work on AST

Eugene Toder added the comment: Is anyone looking or planing to look at the patch? -- ___ Python tracker ___ ___ Python-bugs-list mai

[issue11549] Rewrite peephole to work on AST

Changes by Skip Montanaro : -- nosy: -skip.montanaro ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://ma

[issue11549] Rewrite peephole to work on AST

Changes by Georg Brandl : -- nosy: +georg.brandl ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.py

[issue11549] Rewrite peephole to work on AST

Eugene Toder added the comment: AFAICT my patch has everything that #1346238 has, except BoolOps, which can be easily added (there's a TODO). I don't want to add any new code, though, until the current patch will get reviewed -- adding code will only make reviewing harder. #10399 looks inter

[issue11549] Rewrite peephole to work on AST

Terry J. Reedy added the comment: A couple of somewhat related issues: #10399 AST Optimization: inlining of function calls #1346238 A constant folding optimization pass for the AST Obviously, ast optimizers should work together and not duplicate. Nice to see increased attention. -- nosy

[issue11549] Rewrite peephole to work on AST

Changes by Nadeem Vawda : -- nosy: +nvawda ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.o

[issue11549] Rewrite peephole to work on AST

Changes by Mark Dickinson : -- nosy: +mark.dickinson ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mai

[issue11549] Rewrite peephole to work on AST

Eugene Toder added the comment: Just for fun I've run pystones. W/o my changes it averages to about 70k, with my changes to about 72.5k. -- ___ Python tracker ___ _

[issue11549] Rewrite peephole to work on AST

Eugene Toder added the comment: I've updated patches on Rietveld with some small changes. This includes better code generation for boolops used outside of conditions and cleaned up optimize_jumps(). This is probably the last change before I get some feedback. Also, I forgot to mention yesterda

[issue11549] Rewrite peephole to work on AST

Changes by Alex Gaynor : -- nosy: +alex ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/

[issue11549] Rewrite peephole to work on AST

Nick Coghlan added the comment: I've been focusing on softer targets during the sprints - I'll dig into this once I'm back home and using my desktop machine again. -- ___ Python tracker __

[issue11549] Rewrite peephole to work on AST

Eugene Toder added the comment: Any comments on the code so far or suggestions on how we should move forward? -- ___ Python tracker ___ _

[issue11549] Rewrite peephole to work on AST

Nick Coghlan added the comment: No need, since you manually created a review on appspot. The local Reitveld links are just a convenience that can avoid the need to manually create a review instance. -- ___ Python tracker

[issue11549] Rewrite peephole to work on AST

Eugene Toder added the comment: I see. Should I attach diffs vs. some revision from hg.python.org? -- ___ Python tracker ___ ___ Pyth

[issue11549] Rewrite peephole to work on AST

Nick Coghlan added the comment: The review links didn't come up automatically because 336137a359ae isn't a hg.python.org/cpython revision ID. -- ___ Python tracker ___

[issue11549] Rewrite peephole to work on AST

Eugene Toder added the comment: Thanks. Review link: http://codereview.appspot.com/4281051 -- ___ Python tracker ___ ___ Python-bugs-

[issue11549] Rewrite peephole to work on AST

Antoine Pitrou added the comment: > Because I don't know how to make them. Any pointers? Martin is hacking on the tool these days... So it's no surprise it doesn't work perfectly yet ;) If you have a Google account you can upload these patches to http://codereview.appspot.com/, though. ---

[issue11549] Rewrite peephole to work on AST

Eugene Toder added the comment: Because I don't know how to make them. Any pointers? -- ___ Python tracker ___ ___ Python-bugs-list m

[issue11549] Rewrite peephole to work on AST

Skip Montanaro added the comment: I'm confused. Why aren't there review links? -- nosy: +skip.montanaro ___ Python tracker ___ ___ P

[issue11549] Rewrite peephole to work on AST

Changes by Nick Coghlan : -- nosy: +brett.cannon ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.py

[issue11549] Rewrite peephole to work on AST

Changes by Nick Coghlan : -- nosy: +dmalcolm, ncoghlan ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://m

[issue11549] Rewrite peephole to work on AST

Changes by Daniel Urban : -- nosy: +durban ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.o

[issue11549] Rewrite peephole to work on AST

Changes by Antoine Pitrou : -- nosy: +benjamin.peterson ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://

[issue11549] Rewrite peephole to work on AST

Changes by Santoso Wijaya : -- nosy: +santa4nt ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.pyth

[issue11549] Rewrite peephole to work on AST

Changes by Andreas Stührk : -- nosy: +Trundle ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.pytho

[issue11549] Rewrite peephole to work on AST

Changes by Eugene Toder : -- nosy: +pitrou, rhettinger ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://m

[issue11549] Rewrite peephole to work on AST

Changes by Eugene Toder : Added file: http://bugs.python.org/file21202/0_tests.patch ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue11549] Rewrite peephole to work on AST

Changes by Eugene Toder : Added file: http://bugs.python.org/file21201/0_generated.patch ___ Python tracker ___ ___ Python-bugs-list mailing l

[issue11549] Rewrite peephole to work on AST

Changes by Eugene Toder : Added file: http://bugs.python.org/file21200/0_compile.patch ___ Python tracker ___ ___ Python-bugs-list mailing lis

[issue11549] Rewrite peephole to work on AST

Changes by Eugene Toder : Added file: http://bugs.python.org/file21199/0_fold.patch ___ Python tracker ___ ___ Python-bugs-list mailing list U

[issue11549] Rewrite peephole to work on AST

Changes by Eugene Toder : -- keywords: +patch Added file: http://bugs.python.org/file21198/0_ast.patch ___ Python tracker ___ ___ Pyth

[issue11549] Rewrite peephole to work on AST

New submission from Eugene Toder : As pointed out by Raymond, constant folding should be done on AST rather than on generated bytecode. Here's a patch to do that. It's rather long, so overview first. The patch replaces existing peephole pass with a folding pass on AST and a few changes in com