Wow, this is an excellent review. Thank you. My only question is with respect to this: I think there ought to be two opcodes; one for unpacking maps in function calls and another for literals. The whole function location thing is rather hideous. What are the two opcodes? BUILD_MAP_UNPACK and BUILD_MAP_UNPACK_WITH_CALL? The first takes (n) a number of maps that it will merge, and the second does the same but also accepts (function_call_location) for the purpose of error reporting? Thanks, Neil On Thu, Mar 19, 2015 at 1:53 PM, wrote:

It's a start.

There need to be documentation updates.

There are still unrelated style changes in compile.c that should be reverted.

http://bugs.python.org/review/2292/diff/14152/Include/opcode.h File Include/opcode.h (right):

http://bugs.python.org/review/2292/diff/14152/Include/opcode.h#newcode71 Include/opcode.h:71: #define DELETE_GLOBAL 98 This file should not be manually changed. Rather, Tools/scripts/generate_opcode_h.py should be run.

http://bugs.python.org/review/2292/diff/14152/Lib/importlib/_bootstrap.py File Lib/importlib/_bootstrap.py (right):

http://bugs.python.org/review/2292/diff/14152/Lib/importlib/_bootstrap.py#ne... Lib/importlib/_bootstrap.py:428: MAGIC_NUMBER = (3321).to_bytes(2, 'little') + b'\r\n' As the comment above indicates, the magic value should be incremented by 10 not 1. Also, the comment needs to be updated.

http://bugs.python.org/review/2292/diff/14152/Lib/test/test_ast.py File Lib/test/test_ast.py (right):

http://bugs.python.org/review/2292/diff/14152/Lib/test/test_ast.py#newcode93... Lib/test/test_ast.py:937: main() Why is this here?

http://bugs.python.org/review/2292/diff/14152/Lib/test/test_parser.py File Lib/test/test_parser.py (right):

http://bugs.python.org/review/2292/diff/14152/Lib/test/test_parser.py#newcod... Lib/test/test_parser.py:334: self.check_expr('{**{}, 3:4, **{5:6, 7:8}}') Should there be tests for the new function call syntax, too?

http://bugs.python.org/review/2292/diff/14152/Python/ast.c File Python/ast.c (right):

http://bugs.python.org/review/2292/diff/14152/Python/ast.c#newcode1746 Python/ast.c:1746: ast_error(c, n, "iterable unpacking cannot be used in comprehension"); |n| should probably be |ch|

http://bugs.python.org/review/2292/diff/14152/Python/ast.c#newcode2022 Python/ast.c:2022: if (TYPE(CHILD(ch, 0)) == DOUBLESTAR) int is_dict = TYPE(CHILD(ch, 0)) == DOUBLESTAR;

http://bugs.python.org/review/2292/diff/14152/Python/ast.c#newcode2026 Python/ast.c:2026: && TYPE(CHILD(ch, 1)) == COMMA)) { boolean operators should be on the previous line

http://bugs.python.org/review/2292/diff/14152/Python/ast.c#newcode2031 Python/ast.c:2031: && TYPE(CHILD(ch, 1)) == comp_for) { ditto

http://bugs.python.org/review/2292/diff/14152/Python/ast.c#newcode2036 Python/ast.c:2036: && TYPE(CHILD(ch, 3 - is_dict)) == comp_for) { ditto

http://bugs.python.org/review/2292/diff/14152/Python/ceval.c File Python/ceval.c (right):

http://bugs.python.org/review/2292/diff/14152/Python/ceval.c#newcode2403 Python/ceval.c:2403: as_tuple = PyObject_CallFunctionObjArgs( Use PyList_AsTuple.

http://bugs.python.org/review/2292/diff/14152/Python/ceval.c#newcode2498 Python/ceval.c:2498: TARGET(BUILD_MAP_UNPACK) { I think there ought to be two opcodes; one for unpacking maps in function calls and another for literals. The whole function location thing is rather hideous.

http://bugs.python.org/review/2292/diff/14152/Python/ceval.c#newcode2526 Python/ceval.c:2526: if (PySet_Size(intersection)) { Probably this would be faster with PySet_GET_SIZE(so).

http://bugs.python.org/review/2292/diff/14152/Python/compile.c File Python/compile.c (right):

http://bugs.python.org/review/2292/diff/14152/Python/compile.c#newcode2931 Python/compile.c:2931: asdl_seq_SET(elts, i, elt->v.Starred.value); The compiler should not be mutating the AST.

http://bugs.python.org/review/2292/diff/14152/Python/compile.c#newcode3088 Python/compile.c:3088: int i, nseen, nkw = 0; Many of these should probably be Py_ssize_t.

http://bugs.python.org/review/2292/diff/14152/Python/symtable.c File Python/symtable.c (right):

http://bugs.python.org/review/2292/diff/14152/Python/symtable.c#newcode1372 Python/symtable.c:1372: if (e != NULL) { Please fix the callers, so they don't pass NULL here.

http://bugs.python.org/review/2292/