make list.pop() optimisation integer type safe

Cython/Compiler/Optimize.py

@@ -2503,7 +2503,7 @@ class OptimizeBuiltinCalls(Visitor.MethodDispatcherTransform):
     PyObject_PopIndex_func_type = PyrexTypes.CFuncType(
         PyrexTypes.py_object_type, [
             PyrexTypes.CFuncTypeArg("list", PyrexTypes.py_object_type, None),
-            PyrexTypes.CFuncTypeArg("index", PyrexTypes.c_long_type, None),
+            PyrexTypes.CFuncTypeArg("index", PyrexTypes.c_py_ssize_t_type, None),
             ])
 
     def _handle_simple_method_list_pop(self, node, function, args, is_unbound_method):
@@ -2516,10 +2516,10 @@ class OptimizeBuiltinCalls(Visitor.MethodDispatcherTransform):
         """
         if not args:
             return node
-        args = args[:]
+        obj = args[0]
         if is_list:
             type_name = 'List'
-            args[0] = args[0].as_none_safe_node(
+            obj = obj.as_none_safe_node(
                 "'NoneType' object has no attribute '%s'",
                 error="PyExc_AttributeError",
                 format_args=['pop'])
@@ -2529,28 +2529,37 @@ class OptimizeBuiltinCalls(Visitor.MethodDispatcherTransform):
             return ExprNodes.PythonCapiCallNode(
                 node.pos, "__Pyx_Py%s_Pop" % type_name,
                 self.PyObject_Pop_func_type,
-                args=args,
+                args=[obj],
                 may_return_none=True,
                 is_temp=node.is_temp,
                 utility_code=load_c_utility('pop'),
             )
         elif len(args) == 2:
             index = unwrap_coerced_node(args[1])
-            if is_list or isinstance(index, ExprNodes.IntNode):
-                index = index.coerce_to(PyrexTypes.c_py_ssize_t_type, self.current_env())
-            if index.type.is_int:
-                widest = PyrexTypes.widest_numeric_type(
-                    index.type, PyrexTypes.c_py_ssize_t_type)
-                if widest == PyrexTypes.c_py_ssize_t_type:
-                    args[1] = index
-                    return ExprNodes.PythonCapiCallNode(
-                        node.pos, "__Pyx_Py%s_PopIndex" % type_name,
-                        self.PyObject_PopIndex_func_type,
-                        args=args,
-                        may_return_none=True,
-                        is_temp=node.is_temp,
-                        utility_code=load_c_utility("pop_index"),
-                    )
+            orig_index_type = index.type
+            if not index.type.is_int:
+                if is_list or isinstance(index, ExprNodes.IntNode):
+                    index = index.coerce_to(PyrexTypes.c_py_ssize_t_type, self.current_env())
+                else:
+                    return node
+            elif not PyrexTypes.numeric_type_fits(index.type, PyrexTypes.c_py_ssize_t_type):
+                return node
+            # real type might still be larger at runtime
+            if not orig_index_type.is_int:
+                orig_index_type = index.type
+            if not orig_index_type.create_to_py_utility_code(self.current_env()):
+                return node
+            convert_func = orig_index_type.to_py_function
+            conversion_type = PyrexTypes.CFuncType(
+                PyrexTypes.py_object_type, [PyrexTypes.CFuncTypeArg("intval", orig_index_type, None)])
+            return ExprNodes.PythonCapiCallNode(
+                node.pos, "__Pyx_Py%s_PopIndex" % type_name,
+                self.PyObject_PopIndex_func_type,
+                args=[obj, index, ExprNodes.RawCNameExprNode(index.pos, conversion_type, convert_func)],
+                may_return_none=True,
+                is_temp=node.is_temp,
+                utility_code=load_c_utility("pop_index"),
+            )
 
         return node
 

Cython/Compiler/PyrexTypes.py

@@ -3672,6 +3672,7 @@ def merge_template_deductions(a, b):
             all[param] = value
     return all
 
+
 def widest_numeric_type(type1, type2):
     # Given two numeric types, return the narrowest type
     # encompassing both of them.
@@ -3698,6 +3699,11 @@ def widest_numeric_type(type1, type2):
         widest_type = type2
     return widest_type
 
+
+def numeric_type_fits(small_type, large_type):
+    return widest_numeric_type(small_type, large_type) == large_type
+
+
 def independent_spanning_type(type1, type2):
     # Return a type assignable independently from both type1 and
     # type2, but do not require any interoperability between the two.

Cython/Utility/Optimize.c

@@ -110,25 +110,29 @@ static CYTHON_INLINE PyObject* __Pyx_PyList_Pop(PyObject* L) {
 
 /////////////// pop_index.proto ///////////////
 
-#define __Pyx_PyObject_PopIndex(L, ix) (PyList_CheckExact(L) ? \
-    __Pyx_PyList_PopIndex(L, ix) : __Pyx__PyObject_PopIndex(L, ix))
+#define __Pyx_PyObject_PopIndex(L, ix, to_py_func) ( \
+    (PyList_CheckExact(L) && likely(PY_SSIZE_T_MIN <= ix && ix <= PY_SSIZE_T_MAX)) ? \
+        __Pyx__PyList_PopIndex(L, ix) : __Pyx__PyObject_PopIndex(L, to_py_func(ix)))
 
-static PyObject* __Pyx_PyList_PopIndex(PyObject* L, Py_ssize_t ix); /*proto*/
-static PyObject* __Pyx__PyObject_PopIndex(PyObject* L, Py_ssize_t ix); /*proto*/
+#define __Pyx_PyList_PopIndex(L, ix, to_py_func) ( \
+    likely(PY_SSIZE_T_MIN <= ix && ix <= PY_SSIZE_T_MAX) ? \
+        __Pyx__PyList_PopIndex(L, ix) : __Pyx__PyObject_PopIndex(L, to_py_func(ix)))
+
+static PyObject* __Pyx__PyList_PopIndex(PyObject* L, Py_ssize_t ix); /*proto*/
+static PyObject* __Pyx__PyObject_PopIndex(PyObject* L, PyObject* py_ix); /*proto*/
 
 /////////////// pop_index ///////////////
 //@requires: ObjectHandling.c::PyObjectCallMethod
 
-static PyObject* __Pyx__PyObject_PopIndex(PyObject* L, Py_ssize_t ix) {
-    PyObject *r, *py_ix;
-    py_ix = PyInt_FromSsize_t(ix);
-    if (!py_ix) return NULL;
+static PyObject* __Pyx__PyObject_PopIndex(PyObject* L, PyObject* py_ix) {
+    PyObject *r;
+    if (unlikely(!py_ix)) return NULL;
     r = __Pyx_PyObject_CallMethod1(L, PYIDENT("pop"), py_ix);
     Py_DECREF(py_ix);
     return r;
 }
 
-static PyObject* __Pyx_PyList_PopIndex(PyObject* L, Py_ssize_t ix) {
+static PyObject* __Pyx__PyList_PopIndex(PyObject* L, Py_ssize_t ix) {
 #if CYTHON_COMPILING_IN_CPYTHON
     Py_ssize_t size = PyList_GET_SIZE(L);
     if (likely(size > (((PyListObject*)L)->allocated >> 1))) {
@@ -145,7 +149,7 @@ static PyObject* __Pyx_PyList_PopIndex(PyObject* L, Py_ssize_t ix) {
         }
     }
 #endif
-    return __Pyx__PyObject_PopIndex(L, ix);
+    return __Pyx__PyObject_PopIndex(L, PyInt_FromSsize_t(ix));
 }
 
 

tests/run/list_pop.pyx

 cimport cython
 
+from libc.stdint cimport uint64_t
+
 class A:
     def pop(self, *args):
         print args
@@ -199,3 +201,17 @@ def crazy_pop(L):
     (1, 2, 3)
     """
     return L.pop(1, 2, 3)
+
+
+def object_pop_large_int():
+    """
+    >>> object_pop_large_int()
+    {}
+    """
+    cdef object foo = {}
+    cdef uint64_t bar = 201213467776703617ULL
+
+    foo[bar] = None
+    assert (<object>bar) in foo
+    foo.pop(bar)
+    return foo