bpo-39320: Handle unpacking of **values in compiler

Doc/library/dis.rst

@@ -873,32 +873,25 @@ All of the following opcodes use their arguments.
  .. versionadded:: 3.9
 
 
-.. opcode:: SET_UPDATE
+.. opcode:: SET_UPDATE (i)
 
  Calls ``set.update(TOS1[-i], TOS)``. Used to build sets.
 
  .. versionadded:: 3.9
 
 
-.. opcode:: BUILD_MAP_UNPACK (count)
+.. opcode:: DICT_UPDATE (i)
 
- Pops *count* mappings from the stack, merges them into a single dictionary,
- and pushes the result. Implements dictionary unpacking in dictionary
- displays ``{**x, **y, **z}``.
+ Calls ``dict.update(TOS1[-i], TOS)``. Used to build dicts.
 
- .. versionadded:: 3.5
+ .. versionadded:: 3.9
 
 
-.. opcode:: BUILD_MAP_UNPACK_WITH_CALL (count)
+.. opcode:: DICT_MERGE
 
- This is similar to :opcode:`BUILD_MAP_UNPACK`,
- but is used for ``f(**x, **y, **z)`` call syntax. The stack item at
- position ``count + 2`` should be the corresponding callable ``f``.
+ Like :opcode:`DICT_UPDATE` but raises an exception for duplicate keys.
 
- .. versionadded:: 3.5
- .. versionchanged:: 3.6
- The position of the callable is determined by adding 2 to the opcode
- argument instead of encoding it in the second byte of the argument.
+ .. versionadded:: 3.9
 
 
 .. opcode:: LOAD_ATTR (namei)

Lib/importlib/_bootstrap_external.py

@@ -276,6 +276,7 @@ def _write_atomic(path, data, mode=0o666):
 # Python 3.9a0 3422 (remove BEGIN_FINALLY, END_FINALLY, CALL_FINALLY, POP_FINALLY bytecodes #33387)
 # Python 3.9a2 3423 (add IS_OP, CONTAINS_OP and JUMP_IF_NOT_EXC_MATCH bytecodes #39156)
 # Python 3.9a2 3424 (simplify bytecodes for *value unpacking)
+# Python 3.9a2 3425 (simplify bytecodes for **value unpacking)
 
 #
 # MAGIC must change whenever the bytecode emitted by the compiler may no
@@ -285,7 +286,7 @@ def _write_atomic(path, data, mode=0o666):
 # Whenever MAGIC_NUMBER is changed, the ranges in the magic_values array
 # in PC/launcher.c must also be updated.
 
-MAGIC_NUMBER = (3424).to_bytes(2, 'little') + b'\r\n'
+MAGIC_NUMBER = (3425).to_bytes(2, 'little') + b'\r\n'
 _RAW_MAGIC_NUMBER = int.from_bytes(MAGIC_NUMBER, 'little') # For import.c
 
 _PYCACHE = '__pycache__'

Lib/opcode.py

@@ -200,9 +200,6 @@ def jabs_op(name, op):
 def_op('EXTENDED_ARG', 144)
 EXTENDED_ARG = 144
 
-def_op('BUILD_MAP_UNPACK', 150)
-def_op('BUILD_MAP_UNPACK_WITH_CALL', 151)
-
 jrel_op('SETUP_ASYNC_WITH', 154)
 
 def_op('FORMAT_VALUE', 155)
@@ -214,5 +211,7 @@ def jabs_op(name, op):
 
 def_op('LIST_EXTEND', 162)
 def_op('SET_UPDATE', 163)
+def_op('DICT_MERGE', 164)
+def_op('DICT_UPDATE', 165)
 
 del def_op, name_op, jrel_op, jabs_op

Misc/NEWS.d/next/Core and Builtins/2020-01-15-15-50-22.bpo-39320.oWARyk.rst

@@ -0,0 +1,4 @@
+
+Replace two complex bytecodes for building dicts with two simpler ones.
+The new bytecodes ``DICT_MERGE`` and ``DICT_UPDATE`` have been added
+The old bytecodes ``BUILD_MAP_UNPACK`` and ``BUILD_MAP_UNPACK_WITH_CALL`` have been removed.

Python/ceval.c

@@ -2801,49 +2801,32 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
  DISPATCH();
  }
 
- case TARGET(BUILD_MAP_UNPACK): {
- Py_ssize_t i;
- PyObject *sum = PyDict_New();
- if (sum == NULL)
- goto error;
-
- for (i = oparg; i > 0; i--) {
- PyObject *arg = PEEK(i);
- if (PyDict_Update(sum, arg) < 0) {
- if (_PyErr_ExceptionMatches(tstate, PyExc_AttributeError)) {
- _PyErr_Format(tstate, PyExc_TypeError,
- "'%.200s' object is not a mapping",
- arg->ob_type->tp_name);
- }
- Py_DECREF(sum);
- goto error;
+ case TARGET(DICT_UPDATE): {
+ PyObject *update = POP();
+ PyObject *dict = PEEK(oparg);
+ if (PyDict_Update(dict, update) < 0) {
+ if (_PyErr_ExceptionMatches(tstate, PyExc_AttributeError)) {
+ _PyErr_Format(tstate, PyExc_TypeError,
+ "'%.200s' object is not a mapping",
+ update->ob_type->tp_name);
  }
+ Py_DECREF(update);
+ goto error;
  }
-
- while (oparg--)
- Py_DECREF(POP());
- PUSH(sum);
+ Py_DECREF(update);
  DISPATCH();
  }
 
- case TARGET(BUILD_MAP_UNPACK_WITH_CALL): {
- Py_ssize_t i;
- PyObject *sum = PyDict_New();
- if (sum == NULL)
- goto error;
+ case TARGET(DICT_MERGE): {
+ PyObject *update = POP();
+ PyObject *dict = PEEK(oparg);
 
- for (i = oparg; i > 0; i--) {
- PyObject *arg = PEEK(i);
- if (_PyDict_MergeEx(sum, arg, 2) < 0) {
- Py_DECREF(sum);
- format_kwargs_error(tstate, PEEK(2 + oparg), arg);
- goto error;
- }
+ if (_PyDict_MergeEx(dict, update, 2) < 0) {
+ format_kwargs_error(tstate, PEEK(2 + oparg), update);
+ Py_DECREF(update);
+ goto error;
  }
-
- while (oparg--)
- Py_DECREF(POP());
- PUSH(sum);
+ Py_DECREF(update);
  PREDICT(CALL_FUNCTION_EX);
  DISPATCH();
  }

Python/compile.c

@@ -1007,9 +1007,6 @@ stack_effect(int opcode, int oparg, int jump)
  case BUILD_SET:
  case BUILD_STRING:
  return 1-oparg;
- case BUILD_MAP_UNPACK:
- case BUILD_MAP_UNPACK_WITH_CALL:
- return 1 - oparg;
  case BUILD_MAP:
  return 1 - 2*oparg;
  case BUILD_CONST_KEY_MAP:
@@ -1125,6 +1122,8 @@ stack_effect(int opcode, int oparg, int jump)
  return 0;
  case LIST_EXTEND:
  case SET_UPDATE:
+ case DICT_MERGE:
+ case DICT_UPDATE:
  return -1;
  default:
  return PY_INVALID_STACK_EFFECT;
@@ -3868,37 +3867,58 @@ static int
 compiler_dict(struct compiler *c, expr_ty e)
 {
  Py_ssize_t i, n, elements;
- int containers;
+ int have_dict;
  int is_unpacking = 0;
  n = asdl_seq_LEN(e->v.Dict.values);
- containers = 0;
+ have_dict = 0;
  elements = 0;
  for (i = 0; i < n; i++) {
  is_unpacking = (expr_ty)asdl_seq_GET(e->v.Dict.keys, i) == NULL;
- if (elements == 0xFFFF || (elements && is_unpacking)) {
- if (!compiler_subdict(c, e, i - elements, i))
- return 0;
- containers++;
- elements = 0;
- }
  if (is_unpacking) {
+ if (elements) {
+ if (!compiler_subdict(c, e, i - elements, i)) {
+ return 0;
+ }
+ if (have_dict) {
+ ADDOP_I(c, DICT_UPDATE, 1);
+ }
+ have_dict = 1;
+ elements = 0;
+ }
+ if (have_dict == 0) {
+ ADDOP_I(c, BUILD_MAP, 0);
+ have_dict = 1;
+ }
  VISIT(c, expr, (expr_ty)asdl_seq_GET(e->v.Dict.values, i));
- containers++;
+ ADDOP_I(c, DICT_UPDATE, 1);
  }
  else {
- elements++;
+ if (elements == 0xFFFF) {
+ if (!compiler_subdict(c, e, i - elements, i)) {
+ return 0;
+ }
+ if (have_dict) {
+ ADDOP_I(c, DICT_UPDATE, 1);
+ }
+ have_dict = 1;
+ elements = 0;
+ }
+ else {
+ elements++;
+ }
  }
  }
- if (elements || containers == 0) {
- if (!compiler_subdict(c, e, n - elements, n))
+ if (elements) {
+ if (!compiler_subdict(c, e, n - elements, n)) {
  return 0;
- containers++;
+ }
+ if (have_dict) {
+ ADDOP_I(c, DICT_UPDATE, 1);
+ }
+ have_dict = 1;
  }
- /* If there is more than one dict, they need to be merged into a new
- * dict. If there is one dict and it's an unpacking, then it needs
- * to be copied into a new dict." */
- if (containers > 1 || is_unpacking) {
- ADDOP_I(c, BUILD_MAP_UNPACK, containers);
+ if (!have_dict) {
+ ADDOP_I(c, BUILD_MAP, 0);
  }
  return 1;
 }
@@ -4263,37 +4283,43 @@ compiler_call_helper(struct compiler *c,
  }
  /* Then keyword arguments */
  if (nkwelts) {
- /* the number of dictionaries on the stack */
- Py_ssize_t nsubkwargs = 0;
+ /* Has a new dict been pushed */
+ int have_dict = 0;
 
  nseen = 0; /* the number of keyword arguments on the stack following */
  for (i = 0; i < nkwelts; i++) {
  keyword_ty kw = asdl_seq_GET(keywords, i);
  if (kw->arg == NULL) {
  /* A keyword argument unpacking. */
  if (nseen) {
- if (!compiler_subkwargs(c, keywords, i - nseen, i))
+ if (!compiler_subkwargs(c, keywords, i - nseen, i)) {
  return 0;
- nsubkwargs++;
+ }
+ have_dict = 1;
  nseen = 0;
  }
+ if (!have_dict) {
+ ADDOP_I(c, BUILD_MAP, 0);
+ have_dict = 1;
+ }
  VISIT(c, expr, kw->value);
- nsubkwargs++;
+ ADDOP_I(c, DICT_MERGE, 1);
  }
  else {
  nseen++;
  }
  }
  if (nseen) {
  /* Pack up any trailing keyword arguments. */
- if (!compiler_subkwargs(c, keywords, nkwelts - nseen, nkwelts))
+ if (!compiler_subkwargs(c, keywords, nkwelts - nseen, nkwelts)) {
  return 0;
- nsubkwargs++;
- }
- if (nsubkwargs > 1) {
- /* Pack it all up */
- ADDOP_I(c, BUILD_MAP_UNPACK_WITH_CALL, nsubkwargs);
+ }
+  if (have_dict) {
+  ADDOP_I(c, DICT_MERGE, 1);
+ }
+ have_dict = 1;
  }
+ assert(have_dict);
  }
  ADDOP_I(c, CALL_FUNCTION_EX, nkwelts > 0);
  return 1;