bpo-20180: Use argument clinic for dict.pop() and dict.popitem() (GH-12792)

3 files changed, 101 insertions, 35 deletions

diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-04-12-15-49-15.bpo-20180.KUqVk7.rst b/Misc/NEWS.d/next/Core and Builtins/2019-04-12-15-49-15.bpo-20180.KUqVk7.rst
new file mode 100644
index 0000000..8c90670
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2019-04-12-15-49-15.bpo-20180.KUqVk7.rst
@@ -0,0 +1,2 @@
+``dict.pop()`` is now up to 33% faster thanks to Argument Clinic. Patch by
+Inada Naoki.
diff --git a/Objects/clinic/dictobject.c.h b/Objects/clinic/dictobject.c.h
index 713781c..b87244d 100644
--- a/Objects/clinic/dictobject.c.h
+++ b/Objects/clinic/dictobject.c.h
@@ -116,6 +116,63 @@ exit:
     return return_value;
 }
 
+PyDoc_STRVAR(dict_pop__doc__,
+"pop($self, key, default=None, /)\n"
+"--\n"
+"\n"
+"Remove specified key and return the corresponding value.\n"
+"\n"
+"If key is not found, default is returned if given, otherwise KeyError is raised");
+
+#define DICT_POP_METHODDEF    \
+    {"pop", (PyCFunction)(void(*)(void))dict_pop, METH_FASTCALL, dict_pop__doc__},
+
+static PyObject *
+dict_pop_impl(PyDictObject *self, PyObject *key, PyObject *default_value);
+
+static PyObject *
+dict_pop(PyDictObject *self, PyObject *const *args, Py_ssize_t nargs)
+{
+    PyObject *return_value = NULL;
+    PyObject *key;
+    PyObject *default_value = NULL;
+
+    if (!_PyArg_CheckPositional("pop", nargs, 1, 2)) {
+        goto exit;
+    }
+    key = args[0];
+    if (nargs < 2) {
+        goto skip_optional;
+    }
+    default_value = args[1];
+skip_optional:
+    return_value = dict_pop_impl(self, key, default_value);
+
+exit:
+    return return_value;
+}
+
+PyDoc_STRVAR(dict_popitem__doc__,
+"popitem($self, /)\n"
+"--\n"
+"\n"
+"Remove and return a (key, value) pair as a 2-tuple.\n"
+"\n"
+"Pairs are returned in LIFO (last-in, first-out) order.\n"
+"Raises KeyError if the dict is empty.");
+
+#define DICT_POPITEM_METHODDEF    \
+    {"popitem", (PyCFunction)dict_popitem, METH_NOARGS, dict_popitem__doc__},
+
+static PyObject *
+dict_popitem_impl(PyDictObject *self);
+
+static PyObject *
+dict_popitem(PyDictObject *self, PyObject *Py_UNUSED(ignored))
+{
+    return dict_popitem_impl(self);
+}
+
 PyDoc_STRVAR(dict___reversed____doc__,
 "__reversed__($self, /)\n"
 "--\n"
@@ -133,4 +190,4 @@ dict___reversed__(PyDictObject *self, PyObject *Py_UNUSED(ignored))
 {
     return dict___reversed___impl(self);
 }
-/*[clinic end generated code: output=12c21ce3552d9617 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=0fd5cafc61a51d3c input=a9049054013a1b77]*/
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index c1187c2..9ff009f 100644
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -2985,19 +2985,37 @@ dict_clear(PyDictObject *mp, PyObject *Py_UNUSED(ignored))
     Py_RETURN_NONE;
 }
 
+/*[clinic input]
+dict.pop
+
+    key: object
+    default: object = NULL
+    /
+
+Remove specified key and return the corresponding value.
+
+If key is not found, default is returned if given, otherwise KeyError is raised
+[clinic start generated code]*/
+
 static PyObject *
-dict_pop(PyDictObject *mp, PyObject *args)
+dict_pop_impl(PyDictObject *self, PyObject *key, PyObject *default_value)
+/*[clinic end generated code: output=3abb47b89f24c21c input=016f6a000e4e633b]*/
 {
-    PyObject *key, *deflt = NULL;
+    return _PyDict_Pop((PyObject*)self, key, default_value);
+}
 
-    if(!PyArg_UnpackTuple(args, "pop", 1, 2, &key, &deflt))
-        return NULL;
+/*[clinic input]
+dict.popitem
 
-    return _PyDict_Pop((PyObject*)mp, key, deflt);
-}
+Remove and return a (key, value) pair as a 2-tuple.
+
+Pairs are returned in LIFO (last-in, first-out) order.
+Raises KeyError if the dict is empty.
+[clinic start generated code]*/
 
 static PyObject *
-dict_popitem(PyDictObject *mp, PyObject *Py_UNUSED(ignored))
+dict_popitem_impl(PyDictObject *self)
+/*[clinic end generated code: output=e65fcb04420d230d input=1c38a49f21f64941]*/
 {
     Py_ssize_t i, j;
     PyDictKeyEntry *ep0, *ep;
@@ -3015,44 +3033,43 @@ dict_popitem(PyDictObject *mp, PyObject *Py_UNUSED(ignored))
     res = PyTuple_New(2);
     if (res == NULL)
         return NULL;
-    if (mp->ma_used == 0) {
+    if (self->ma_used == 0) {
         Py_DECREF(res);
-        PyErr_SetString(PyExc_KeyError,
-                        "popitem(): dictionary is empty");
+        PyErr_SetString(PyExc_KeyError, "popitem(): dictionary is empty");
         return NULL;
     }
     /* Convert split table to combined table */
-    if (mp->ma_keys->dk_lookup == lookdict_split) {
-        if (dictresize(mp, DK_SIZE(mp->ma_keys))) {
+    if (self->ma_keys->dk_lookup == lookdict_split) {
+        if (dictresize(self, DK_SIZE(self->ma_keys))) {
             Py_DECREF(res);
             return NULL;
         }
     }
-    ENSURE_ALLOWS_DELETIONS(mp);
+    ENSURE_ALLOWS_DELETIONS(self);
 
     /* Pop last item */
-    ep0 = DK_ENTRIES(mp->ma_keys);
-    i = mp->ma_keys->dk_nentries - 1;
+    ep0 = DK_ENTRIES(self->ma_keys);
+    i = self->ma_keys->dk_nentries - 1;
     while (i >= 0 && ep0[i].me_value == NULL) {
         i--;
     }
     assert(i >= 0);
 
     ep = &ep0[i];
-    j = lookdict_index(mp->ma_keys, ep->me_hash, i);
+    j = lookdict_index(self->ma_keys, ep->me_hash, i);
     assert(j >= 0);
-    assert(dictkeys_get_index(mp->ma_keys, j) == i);
-    dictkeys_set_index(mp->ma_keys, j, DKIX_DUMMY);
+    assert(dictkeys_get_index(self->ma_keys, j) == i);
+    dictkeys_set_index(self->ma_keys, j, DKIX_DUMMY);
 
     PyTuple_SET_ITEM(res, 0, ep->me_key);
     PyTuple_SET_ITEM(res, 1, ep->me_value);
     ep->me_key = NULL;
     ep->me_value = NULL;
     /* We can't dk_usable++ since there is DKIX_DUMMY in indices */
-    mp->ma_keys->dk_nentries = i;
-    mp->ma_used--;
-    mp->ma_version_tag = DICT_NEXT_VERSION();
-    assert(_PyDict_CheckConsistency(mp));
+    self->ma_keys->dk_nentries = i;
+    self->ma_used--;
+    self->ma_version_tag = DICT_NEXT_VERSION();
+    assert(_PyDict_CheckConsistency(self));
     return res;
 }
 
@@ -3135,14 +3152,6 @@ PyDoc_STRVAR(getitem__doc__, "x.__getitem__(y) <==> x[y]");
 PyDoc_STRVAR(sizeof__doc__,
 "D.__sizeof__() -> size of D in memory, in bytes");
 
-PyDoc_STRVAR(pop__doc__,
-"D.pop(k[,d]) -> v, remove specified key and return the corresponding value.\n\
-If key is not found, d is returned if given, otherwise KeyError is raised");
-
-PyDoc_STRVAR(popitem__doc__,
-"D.popitem() -> (k, v), remove and return some (key, value) pair as a\n\
-2-tuple; but raise KeyError if D is empty.");
-
 PyDoc_STRVAR(update__doc__,
 "D.update([E, ]**F) -> None.  Update D from dict/iterable E and F.\n\
 If E is present and has a .keys() method, then does:  for k in E: D[k] = E[k]\n\
@@ -3175,10 +3184,8 @@ static PyMethodDef mapp_methods[] = {
      sizeof__doc__},
     DICT_GET_METHODDEF
     DICT_SETDEFAULT_METHODDEF
-    {"pop",         (PyCFunction)dict_pop,          METH_VARARGS,
-     pop__doc__},
-    {"popitem",         (PyCFunction)(void(*)(void))dict_popitem,      METH_NOARGS,
-     popitem__doc__},
+    DICT_POP_METHODDEF
+    DICT_POPITEM_METHODDEF
     {"keys",            dictkeys_new,                   METH_NOARGS,
     keys__doc__},
     {"items",           dictitems_new,                  METH_NOARGS,