Issue #2183: Simplify and optimize bytecode for list comprehensions.

diff --git a/Doc/library/dis.rst b/Doc/library/dis.rst
index 84a0c2598b5c57284d63507723a38ee629fc39d2..e9cbb131ea9c3d4a48db206f9808100d6e1cfb9f 100644 (file)
--- a/Doc/library/dis.rst
+++ b/Doc/library/dis.rst
@@ -463,9 +463,11 @@ Miscellaneous opcodes.
    address to jump to (which should be a ``FOR_ITER`` instruction).
 
 
-.. opcode:: LIST_APPEND ()
+.. opcode:: LIST_APPEND (i)
 
-   Calls ``list.append(TOS1, TOS)``.  Used to implement list comprehensions.
+   Calls ``list.append(TOS[-i], TOS)``.  Used to implement list comprehensions.
+   While the appended value is popped off, the list object remains on the
+   stack so that it is available for further iterations of the loop.
 
 
 .. opcode:: LOAD_LOCALS ()

diff --git a/Include/opcode.h b/Include/opcode.h
index 9f20b56d6851e64846d9481e7961a918925f1904..5e6fba088c8a6a70685767d7abf8683676cfe3b1 100644 (file)
--- a/Include/opcode.h
+++ b/Include/opcode.h
@@ -22,7 +22,6 @@ extern "C" {
 
 #define UNARY_INVERT   15
 
-#define LIST_APPEND    18
 #define BINARY_POWER   19
 
 #define BINARY_MULTIPLY        20
@@ -89,6 +88,7 @@ extern "C" {
 #define DELETE_NAME    91      /* "" */
 #define UNPACK_SEQUENCE        92      /* Number of sequence items */
 #define FOR_ITER       93
+#define LIST_APPEND    94
 
 #define STORE_ATTR     95      /* Index in name list */
 #define DELETE_ATTR    96      /* "" */

diff --git a/Lib/compiler/pycodegen.py b/Lib/compiler/pycodegen.py
index 61b9fe9bb6edc62375886b334cadfcd96e8a4e63..5d5dca0d99081fa51ca7170f48d02d3a488b9548 100644 (file)
--- a/Lib/compiler/pycodegen.py
+++ b/Lib/compiler/pycodegen.py
@@ -570,16 +570,10 @@ class CodeGenerator:
             self.nextBlock(end)
 
     # list comprehensions
-    __list_count = 0
-
     def visitListComp(self, node):
         self.set_lineno(node)
         # setup list
-        tmpname = "$list%d" % self.__list_count
-        self.__list_count = self.__list_count + 1
         self.emit('BUILD_LIST', 0)
-        self.emit('DUP_TOP')
-        self._implicitNameOp('STORE', tmpname)
 
         stack = []
         for i, for_ in zip(range(len(node.quals)), node.quals):
@@ -591,9 +585,8 @@ class CodeGenerator:
                 self.visit(if_, cont)
             stack.insert(0, (start, cont, anchor))
 
-        self._implicitNameOp('LOAD', tmpname)
         self.visit(node.expr)
-        self.emit('LIST_APPEND')
+        self.emit('LIST_APPEND', len(node.quals) + 1)
 
         for start, cont, anchor in stack:
             if cont:
@@ -604,9 +597,6 @@ class CodeGenerator:
                 self.nextBlock(skip_one)
             self.emit('JUMP_ABSOLUTE', start)
             self.startBlock(anchor)
-        self._implicitNameOp('DELETE', tmpname)
-
-        self.__list_count = self.__list_count - 1
 
     def visitListCompFor(self, node):
         start = self.newBlock()

diff --git a/Lib/opcode.py b/Lib/opcode.py
index cee5057f919ceeb6c856b82339f0e57bf6fc31d6..d41383b2f6f9bc97c9ee61ebb93859aa708fb128 100644 (file)
--- a/Lib/opcode.py
+++ b/Lib/opcode.py
@@ -58,7 +58,6 @@ def_op('UNARY_CONVERT', 13)
 
 def_op('UNARY_INVERT', 15)
 
-def_op('LIST_APPEND', 18)
 def_op('BINARY_POWER', 19)
 def_op('BINARY_MULTIPLY', 20)
 def_op('BINARY_DIVIDE', 21)
@@ -128,7 +127,7 @@ name_op('STORE_NAME', 90)       # Index in name list
 name_op('DELETE_NAME', 91)      # ""
 def_op('UNPACK_SEQUENCE', 92)   # Number of tuple items
 jrel_op('FOR_ITER', 93)
-
+def_op('LIST_APPEND', 94)
 name_op('STORE_ATTR', 95)       # Index in name list
 name_op('DELETE_ATTR', 96)      # ""
 name_op('STORE_GLOBAL', 97)     # ""

diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py
index fd508a94a26000776fbc6b6b64a70722d5e7e3f9..6fa437b76e05b0a263d1f8b201cac4c1d7ea1e40 100644 (file)
--- a/Lib/test/test_dis.py
+++ b/Lib/test/test_dis.py
@@ -54,29 +54,25 @@ def bug1333982(x=[]):
 
 dis_bug1333982 = """\
  %-4d         0 LOAD_CONST               1 (0)
-              3 JUMP_IF_TRUE            41 (to 47)
+              3 JUMP_IF_TRUE            33 (to 39)
               6 POP_TOP
               7 LOAD_GLOBAL              0 (AssertionError)
              10 BUILD_LIST               0
-             13 DUP_TOP
-             14 STORE_FAST               1 (_[1])
-             17 LOAD_FAST                0 (x)
-             20 GET_ITER
-        >>   21 FOR_ITER                13 (to 37)
-             24 STORE_FAST               2 (s)
-             27 LOAD_FAST                1 (_[1])
-             30 LOAD_FAST                2 (s)
-             33 LIST_APPEND
-             34 JUMP_ABSOLUTE           21
-        >>   37 DELETE_FAST              1 (_[1])
-
- %-4d        40 LOAD_CONST               2 (1)
-             43 BINARY_ADD
-             44 RAISE_VARARGS            2
-        >>   47 POP_TOP
-
- %-4d        48 LOAD_CONST               0 (None)
-             51 RETURN_VALUE
+             13 LOAD_FAST                0 (x)
+             16 GET_ITER
+        >>   17 FOR_ITER                12 (to 32)
+             20 STORE_FAST               1 (s)
+             23 LOAD_FAST                1 (s)
+             26 LIST_APPEND              2
+             29 JUMP_ABSOLUTE           17
+
+ %-4d   >>   32 LOAD_CONST               2 (1)
+             35 BINARY_ADD
+             36 RAISE_VARARGS            2
+        >>   39 POP_TOP
+
+ %-4d        40 LOAD_CONST               0 (None)
+             43 RETURN_VALUE
 """%(bug1333982.func_code.co_firstlineno + 1,
      bug1333982.func_code.co_firstlineno + 2,
      bug1333982.func_code.co_firstlineno + 3)

diff --git a/Misc/NEWS b/Misc/NEWS
index 9567505b4ba65cef7963aa995957e99dda271750..9e499ae5af93d78df9af73b1b923ac7be28488ec 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,6 +12,9 @@ What's New in Python 2.7 alpha 1
 Core and Builtins
 -----------------
 
+- Issue #2183: Simplify and optimize bytecode for list comprehensions.
+  Original patch by Neal Norwitz.
+
 - Issue #4597: Fixed exception handling when the __exit__ function of a
   context manager returns a value that cannot be converted to a bool.
 

diff --git a/Python/ceval.c b/Python/ceval.c
index bd35185c8464ccf1149c3ce4e464817a57ef0a70..09501afe6e44bc752725f5f4f350a6569f7cb9c1 100644 (file)
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -1294,9 +1294,8 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag)
 
                case LIST_APPEND:
                        w = POP();
-                       v = POP();
+                       v = stack_pointer[-oparg];
                        err = PyList_Append(v, w);
-                       Py_DECREF(v);
                        Py_DECREF(w);
                        if (err == 0) {
                                PREDICT(JUMP_ABSOLUTE);

diff --git a/Python/compile.c b/Python/compile.c
index 88d54ab9445a51e9f52c1c7741621ccf9d852b6d..756a903340827f26a10abf0df29224a279159bea 100644 (file)
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -693,7 +693,7 @@ opcode_stack_effect(int opcode, int oparg)
                        return 0;
 
                case LIST_APPEND:
-                       return -2;
+                       return -1;
 
                case BINARY_POWER:
                case BINARY_MULTIPLY:
@@ -2599,9 +2599,8 @@ compiler_call(struct compiler *c, expr_ty e)
 }
 
 static int
-compiler_listcomp_generator(struct compiler *c, PyObject *tmpname,
-                           asdl_seq *generators, int gen_index, 
-                           expr_ty elt)
+compiler_listcomp_generator(struct compiler *c, asdl_seq *generators,
+                           int gen_index, expr_ty elt)
 {
        /* generate code for the iterator, then each of the ifs,
           and then write to the element */
@@ -2638,16 +2637,13 @@ compiler_listcomp_generator(struct compiler *c, PyObject *tmpname,
        } 
 
        if (++gen_index < asdl_seq_LEN(generators))
-           if (!compiler_listcomp_generator(c, tmpname, 
-                                            generators, gen_index, elt))
+           if (!compiler_listcomp_generator(c, generators, gen_index, elt))
                return 0;
 
        /* only append after the last for generator */
        if (gen_index >= asdl_seq_LEN(generators)) {
-           if (!compiler_nameop(c, tmpname, Load))
-               return 0;
            VISIT(c, expr, elt);
-           ADDOP(c, LIST_APPEND);
+           ADDOP_I(c, LIST_APPEND, gen_index+1);
 
            compiler_use_next_block(c, skip);
        }
@@ -2659,10 +2655,6 @@ compiler_listcomp_generator(struct compiler *c, PyObject *tmpname,
        } 
        ADDOP_JABS(c, JUMP_ABSOLUTE, start);
        compiler_use_next_block(c, anchor);
-       /* delete the temporary list name added to locals */
-       if (gen_index == 1)
-           if (!compiler_nameop(c, tmpname, Del))
-               return 0;
        
        return 1;
 }
@@ -2670,21 +2662,10 @@ compiler_listcomp_generator(struct compiler *c, PyObject *tmpname,
 static int
 compiler_listcomp(struct compiler *c, expr_ty e)
 {
-       identifier tmp;
-       int rc = 0;
-       asdl_seq *generators = e->v.ListComp.generators;
-
        assert(e->kind == ListComp_kind);
-       tmp = compiler_new_tmpname(c);
-       if (!tmp)
-               return 0;
        ADDOP_I(c, BUILD_LIST, 0);
-       ADDOP(c, DUP_TOP);
-       if (compiler_nameop(c, tmp, Store))
-           rc = compiler_listcomp_generator(c, tmp, generators, 0, 
-                                            e->v.ListComp.elt);
-       Py_DECREF(tmp);
-       return rc;
+       return compiler_listcomp_generator(c, e->v.ListComp.generators, 0,
+                                          e->v.ListComp.elt);
 }
 
 static int

diff --git a/Python/import.c b/Python/import.c
index 781bb48c6dc5c63c0610933dab1258d4aa11171b..40fc0186e8dc8975715e2a901d77c4ea9bcb5c62 100644 (file)
--- a/Python/import.c
+++ b/Python/import.c
@@ -73,9 +73,10 @@ extern time_t PyOS_GetLastModificationTime(char *, FILE *);
        Python 2.5c2: 62131 (fix wrong code: for x, in ... in listcomp/genexp)
        Python 2.6a0: 62151 (peephole optimizations and STORE_MAP opcode)
        Python 2.6a1: 62161 (WITH_CLEANUP optimization)
+       Python 2.7a0: 62171 (optimize list comprehensions/change LIST_APPEND)
 .
 */
-#define MAGIC (62161 | ((long)'\r'<<16) | ((long)'\n'<<24))
+#define MAGIC (62171 | ((long)'\r'<<16) | ((long)'\n'<<24))
 
 /* Magic word as global; note that _PyImport_Init() can change the
    value of this global to accommodate for alterations of how the