Improve optimisation of "dict.pop()"

Cython/Compiler/Optimize.py

@@ -3367,21 +3367,40 @@ def _handle_simple_method_dict_setdefault(self, node, function, args, is_unbound
             PyrexTypes.CFuncTypeArg("default", PyrexTypes.py_object_type, None),
             ])
 
+    PyDict_Pop_ignore_func_type = PyrexTypes.CFuncType(
+        PyrexTypes.c_int_type, [
+            PyrexTypes.CFuncTypeArg("dict", PyrexTypes.py_object_type, None),
+            PyrexTypes.CFuncTypeArg("key", PyrexTypes.py_object_type, None),
+            PyrexTypes.CFuncTypeArg("default", PyrexTypes.py_object_type, None),
+            ],
+            exception_value=PyrexTypes.c_int_type.exception_value,
+    )
+
     def _handle_simple_method_dict_pop(self, node, function, args, is_unbound_method):
         """Replace dict.pop() by a call to _PyDict_Pop().
         """
+        capi_func = "__Pyx_PyDict_Pop"
+        utility_code_name = 'py_dict_pop'
+        func_type = self.PyDict_Pop_func_type
+
         if len(args) == 2:
             args.append(ExprNodes.NullNode(node.pos))
-        elif len(args) != 3:
+        elif len(args) == 3:
+            if not node.result_is_used:
+                # special case: we can ignore the default value
+                capi_func = "__Pyx_PyDict_Pop_ignore"
+                utility_code_name = 'py_dict_pop_ignore'
+                func_type = self.PyDict_Pop_ignore_func_type
+        else:
             self._error_wrong_arg_count('dict.pop', node, args, "2 or 3")
             return node
 
         return self._substitute_method_call(
             node, function,
-            "__Pyx_PyDict_Pop", self.PyDict_Pop_func_type,
+            capi_func, func_type,
             'pop', is_unbound_method, args,
             may_return_none=True,
-            utility_code=load_c_utility('py_dict_pop'))
+            utility_code=load_c_utility(utility_code_name))
 
     Pyx_BinopInt_func_types = {
         (ctype, ret_type): PyrexTypes.CFuncType(

Cython/Utility/ObjectHandling.c

@@ -1780,7 +1780,9 @@ static int __Pyx_TryUnpackUnboundCMethod(__Pyx_CachedCFunction* target) {
 /////////////// CallUnboundCMethod0.proto ///////////////
 //@substitute: naming
 
+CYTHON_UNUSED
 static PyObject* __Pyx__CallUnboundCMethod0(__Pyx_CachedCFunction* cfunc, PyObject* self); /*proto*/
+
 #if CYTHON_COMPILING_IN_CPYTHON
 // FASTCALL methods receive "&empty_tuple" as simple "PyObject[0]*"
 #define __Pyx_CallUnboundCMethod0(cfunc, self)  \
@@ -1823,6 +1825,7 @@ static PyObject* __Pyx__CallUnboundCMethod0(__Pyx_CachedCFunction* cfunc, PyObje
 
 /////////////// CallUnboundCMethod1.proto ///////////////
 
+CYTHON_UNUSED
 static PyObject* __Pyx__CallUnboundCMethod1(__Pyx_CachedCFunction* cfunc, PyObject* self, PyObject* arg);/*proto*/
 
 #if CYTHON_COMPILING_IN_CPYTHON
@@ -1886,6 +1889,7 @@ static PyObject* __Pyx__CallUnboundCMethod1(__Pyx_CachedCFunction* cfunc, PyObje
 
 /////////////// CallUnboundCMethod2.proto ///////////////
 
+CYTHON_UNUSED
 static PyObject* __Pyx__CallUnboundCMethod2(__Pyx_CachedCFunction* cfunc, PyObject* self, PyObject* arg1, PyObject* arg2); /*proto*/
 
 #if CYTHON_COMPILING_IN_CPYTHON

Cython/Utility/Optimize.c

@@ -263,17 +263,57 @@ static CYTHON_INLINE PyObject *__Pyx_PyDict_Pop(PyObject *d, PyObject *key, PyOb
 /////////////// py_dict_pop ///////////////
 
 static CYTHON_INLINE PyObject *__Pyx_PyDict_Pop(PyObject *d, PyObject *key, PyObject *default_value) {
-#if CYTHON_COMPILING_IN_CPYTHON && PY_VERSION_HEX < 0x030d0000
-    if ((1)) {
-        return _PyDict_Pop(d, key, default_value);
-    } else
-    // avoid "function unused" warnings
-#endif
+#if PY_VERSION_HEX >= 0x030d00A2
+    PyObject *value;
+    if (PyDict_Pop(d, key, &value) == 0) {
+        if (default_value) {
+            Py_INCREF(default_value);
+        } else {
+            PyErr_SetObject(PyExc_KeyError, key);
+        }
+        value = default_value;
+    }
+    // On error, PyDict_Pop() returns -1 and sets value to NULL (our own exception return value).
+    return value;
+#elif CYTHON_COMPILING_IN_CPYTHON && PY_VERSION_HEX < 0x030d0000
+    return _PyDict_Pop(d, key, default_value);
+#else
     if (default_value) {
         return CALL_UNBOUND_METHOD(PyDict_Type, "pop", d, key, default_value);
     } else {
         return CALL_UNBOUND_METHOD(PyDict_Type, "pop", d, key);
     }
+#endif
+}
+
+
+/////////////// py_dict_pop_ignore.proto ///////////////
+
+static CYTHON_INLINE int __Pyx_PyDict_Pop_ignore(PyObject *d, PyObject *key, PyObject *default_value); /*proto*/
+
+/////////////// py_dict_pop_ignore ///////////////
+
+static CYTHON_INLINE int __Pyx_PyDict_Pop_ignore(PyObject *d, PyObject *key, PyObject *default_value) {
+    // We take the "default_value" as argument to avoid "unused" warnings, but we ignore it here.
+#if PY_VERSION_HEX >= 0x030d00A2
+    int result = PyDict_Pop(d, key, NULL);
+    CYTHON_UNUSED_VAR(default_value);
+    return (unlikely(result == -1)) ? -1 : 0;
+#else
+    PyObject *value;
+    CYTHON_UNUSED_VAR(default_value);
+
+    #if CYTHON_COMPILING_IN_CPYTHON && PY_VERSION_HEX < 0x030d0000
+    value = _PyDict_Pop(d, key, Py_None);
+    #else
+    value = CALL_UNBOUND_METHOD(PyDict_Type, "pop", d, key, Py_None);
+    #endif
+
+    if (unlikely(value == NULL))
+        return -1;
+    Py_DECREF(value);
+    return 0;
+#endif
 }
 
 

tests/run/dict_pop.pyx

@@ -3,13 +3,23 @@
 
 cimport cython
 
+class FailHash:
+    def __hash__(self):
+        raise TypeError()
+
+
 @cython.test_assert_path_exists("//PythonCapiCallNode")
 @cython.test_fail_if_path_exists("//AttributeNode")
 def dict_pop(dict d, key):
     """
     >>> d = { 1: 10, 2: 20 }
     >>> dict_pop(d, 1)
     (10, {2: 20})
+    >>> dict_pop(d, FailHash())
+    Traceback (most recent call last):
+    TypeError
+    >>> d
+    {2: 20}
     >>> dict_pop(d, 3)
     Traceback (most recent call last):
     KeyError: 3
@@ -26,6 +36,11 @@ def dict_pop_default(dict d, key, default):
     >>> d = { 1: 10, 2: 20 }
     >>> dict_pop_default(d, 1, "default")
     (10, {2: 20})
+    >>> dict_pop_default(d, FailHash(), 30)
+    Traceback (most recent call last):
+    TypeError
+    >>> d
+    {2: 20}
     >>> dict_pop_default(d, 3, None)
     (None, {2: 20})
     >>> dict_pop_default(d, 3, "default")
@@ -36,6 +51,26 @@ def dict_pop_default(dict d, key, default):
     return d.pop(key, default), d
 
 
+@cython.test_assert_path_exists("//PythonCapiCallNode")
+@cython.test_fail_if_path_exists("//AttributeNode")
+def dict_pop_ignored(dict d, key):
+    """
+    >>> d = {1: 2, 'a': 'b'}
+    >>> dict_pop_ignored(d, 'a')
+    >>> d
+    {1: 2}
+    >>> dict_pop_ignored(d, FailHash())
+    Traceback (most recent call last):
+    TypeError
+    >>> d
+    {1: 2}
+    >>> dict_pop_ignored(d, 123)
+    >>> d
+    {1: 2}
+    """
+    d.pop(key, None)
+
+
 cdef class MyType:
     cdef public int i
     def __init__(self, i):