Address reviews

Author: vstinner

Doc/c-api/list.rst

@@ -54,7 +54,7 @@ List Objects
 .. c:function:: int PyList_Clear(PyObject *list)
 
    Remove all items from *list*. Similar to:
-   ``PyList_SetSlice(L, 0, PY_SSIZE_T_MAX, NULL)``.
+   ``PyList_SetSlice(list, 0, PY_SSIZE_T_MAX, NULL)``.
 
    Raise an exception and return ``-1`` if *list* is not a :class:`list`
    object.  Return 0 on success.
@@ -123,10 +123,10 @@ List Objects
    to ``list.append(item)``.
 
 
-.. c:function:: int PyList_Extend(PyObject *self, PyObject *iterable)
+.. c:function:: int PyList_Extend(PyObject *list, PyObject *iterable)
 
    Extend *list* with the contents of *iterable*. Similar to:
-   ``PyList_SetSlice(L, PY_SSIZE_T_MAX, PY_SSIZE_T_MAX, iterable)``.
+   ``PyList_SetSlice(list, PY_SSIZE_T_MAX, PY_SSIZE_T_MAX, iterable)``.
 
    Raise an exception and return ``-1`` if *list* is not a :class:`list`
    object. Return 0 on success.

Lib/test/test_capi/test_list.py

@@ -297,15 +297,16 @@ def test_list_extend(self):
         # Test PyList_Extend()
         list_extend = _testcapi.list_extend
 
-        lst = [1]
-        arg = [2, 3]
-        self.assertEqual(list_extend(lst, arg), 0)
-        self.assertEqual(lst, [1, 2, 3])
+        for other_type in (list, tuple, str, iter):
+            lst = list("ab")
+            arg = other_type("def")
+            self.assertEqual(list_extend(lst, arg), 0)
+            self.assertEqual(lst, list("abdef"))
 
-        lst = []
-        arg = (2, 3)
-        self.assertEqual(list_extend(lst, arg), 0)
-        self.assertEqual(lst, [2, 3])
+        # PyList_Extend(lst, lst)
+        lst = list("abc")
+        self.assertEqual(list_extend(lst, lst), 0)
+        self.assertEqual(lst, list("abcabc"))
 
         self.assertRaises(TypeError, list_extend, [], object())
 

Objects/listobject.c

@@ -857,19 +857,13 @@ list_append(PyListObject *self, PyObject *object)
 static int
 list_extend_fast(PyListObject *self, PyObject *iterable)
 {
-    iterable = PySequence_Fast(iterable, "argument must be iterable");
-    if (!iterable) {
-        return -1;
-    }
-
-    Py_ssize_t n = PySequence_Fast_GET_SIZE(iterable);  // size of iterable
+    Py_ssize_t n = PySequence_Fast_GET_SIZE(iterable);
     if (n == 0) {
         /* short circuit when iterable is empty */
-        Py_DECREF(iterable);
         return 0;
     }
 
-    Py_ssize_t m = Py_SIZE(self); // size of self
+    Py_ssize_t m = Py_SIZE(self);
     // It should not be possible to allocate a list large enough to cause
     // an overflow on any relevant platform.
     assert(m < PY_SSIZE_T_MAX - n);
@@ -880,7 +874,6 @@ list_extend_fast(PyListObject *self, PyObject *iterable)
         Py_SET_SIZE(self, n);
     }
     else if (list_resize(self, m + n) < 0) {
-        Py_DECREF(iterable);
         return -1;
     }
 
@@ -896,28 +889,26 @@ list_extend_fast(PyListObject *self, PyObject *iterable)
         PyObject *o = src[i];
         dest[i] = Py_NewRef(o);
     }
-    Py_DECREF(iterable);
     return 0;
 }
 
 static int
 list_extend_iter(PyListObject *self, PyObject *iterable)
 {
-
-    PyObject *it = PyObject_GetIter(iterable);  // iter(v)
+    PyObject *it = PyObject_GetIter(iterable);
     if (it == NULL) {
         return -1;
     }
     PyObject *(*iternext)(PyObject *) = *Py_TYPE(it)->tp_iternext;
 
     /* Guess a result list size. */
-    Py_ssize_t n = PyObject_LengthHint(iterable, 8);  // guess iterable size
+    Py_ssize_t n = PyObject_LengthHint(iterable, 8);
     if (n < 0) {
         Py_DECREF(it);
         return -1;
     }
 
-    Py_ssize_t m = Py_SIZE(self); /* size of self */
+    Py_ssize_t m = Py_SIZE(self);
     if (m > PY_SSIZE_T_MAX - n) {
         /* m + n overflowed; on the chance that n lied, and there really
          * is enough room, ignore it.  If n was telling the truth, we'll
@@ -930,8 +921,10 @@ list_extend_iter(PyListObject *self, PyObject *iterable)
     }
     else {
         /* Make room. */
-        if (list_resize(self, m + n) < 0)
+        if (list_resize(self, m + n) < 0) {
             goto error;
+        }
+
         /* Make the list sane again. */
         Py_SET_SIZE(self, m);
     }
@@ -948,10 +941,10 @@ list_extend_iter(PyListObject *self, PyObject *iterable)
             }
             break;
         }
+
         if (Py_SIZE(self) < self->allocated) {
-            /* steals ref */
             Py_ssize_t len = Py_SIZE(self);
-            PyList_SET_ITEM(self, len, item);
+            PyList_SET_ITEM(self, len, item);  // steals item ref
             Py_SET_SIZE(self, len + 1);
         }
         else {
@@ -974,6 +967,7 @@ list_extend_iter(PyListObject *self, PyObject *iterable)
     return -1;
 }
 
+
 static int
 list_extend(PyListObject *self, PyObject *iterable)
 {
@@ -984,22 +978,31 @@ list_extend(PyListObject *self, PyObject *iterable)
         || PyTuple_CheckExact(iterable)
         || (PyObject *)self == iterable)
     {
-        return list_extend_fast(self, iterable);
+        iterable = PySequence_Fast(iterable, "argument must be iterable");
+        if (!iterable) {
+            return -1;
+        }
+
+        int res = list_extend_fast(self, iterable);
+        Py_DECREF(iterable);
+        return res;
     }
     else {
         return list_extend_iter(self, iterable);
     }
 }
 
-static PyObject *
-list_extend_method(PyListObject *self, PyObject *iterable)
+
+PyObject *
+_PyList_Extend(PyListObject *self, PyObject *iterable)
 {
     if (list_extend(self, iterable) < 0) {
         return NULL;
     }
     Py_RETURN_NONE;
 }
 
+
 /*[clinic input]
 list.extend as py_list_extend
 
@@ -1013,14 +1016,9 @@ static PyObject *
 py_list_extend(PyListObject *self, PyObject *iterable)
 /*[clinic end generated code: output=b8e0bff0ceae2abd input=9a8376a8633ed3ba]*/
 {
-    return list_extend_method(self, iterable);
+    return _PyList_Extend(self, iterable);
 }
 
-PyObject *
-_PyList_Extend(PyListObject *self, PyObject *iterable)
-{
-    return list_extend_method(self, iterable);
-}
 
 int
 PyList_Extend(PyObject *self, PyObject *iterable)
@@ -1033,6 +1031,7 @@ PyList_Extend(PyObject *self, PyObject *iterable)
     return list_extend((PyListObject*)self, iterable);
 }
 
+
 int
 PyList_Clear(PyObject *self)
 {
@@ -1045,6 +1044,7 @@ PyList_Clear(PyObject *self)
     return 0;
 }
 
+
 static PyObject *
 list_inplace_concat(PyListObject *self, PyObject *other)
 {