[Python-checkins] bpo-28280: Make PyMapping_Keys(), PyMapping_Values() and PyMapping_Items() always return a list (#3840)
Serhiy Storchaka
webhook-mailer at python.org
Sun Oct 8 04:17:49 EDT 2017
https://github.com/python/cpython/commit/0ccc0f6c7495be9043300e22d8f38e6d65e8884f
commit: 0ccc0f6c7495be9043300e22d8f38e6d65e8884f
branch: master
author: Oren Milman <orenmn at gmail.com>
committer: Serhiy Storchaka <storchaka at gmail.com>
date: 2017-10-08T11:17:46+03:00
summary:
bpo-28280: Make PyMapping_Keys(), PyMapping_Values() and PyMapping_Items() always return a list (#3840)
files:
A Misc/NEWS.d/next/C API/2017-09-30-19-41-44.bpo-28280.K_EjpO.rst
M Doc/c-api/mapping.rst
M Doc/whatsnew/3.7.rst
M Lib/test/test_capi.py
M Modules/_testcapimodule.c
M Objects/abstract.c
diff --git a/Doc/c-api/mapping.rst b/Doc/c-api/mapping.rst
index a71e9428377..308a9761f87 100644
--- a/Doc/c-api/mapping.rst
+++ b/Doc/c-api/mapping.rst
@@ -50,20 +50,29 @@ Mapping Protocol
.. c:function:: PyObject* PyMapping_Keys(PyObject *o)
- On success, return a list or tuple of the keys in object *o*. On failure,
- return *NULL*.
+ On success, return a list of the keys in object *o*. On failure, return
+ *NULL*.
+
+ .. versionchanged:: 3.7
+ Previously, the function returned a list or a tuple.
.. c:function:: PyObject* PyMapping_Values(PyObject *o)
- On success, return a list or tuple of the values in object *o*. On failure,
- return *NULL*.
+ On success, return a list of the values in object *o*. On failure, return
+ *NULL*.
+
+ .. versionchanged:: 3.7
+ Previously, the function returned a list or a tuple.
.. c:function:: PyObject* PyMapping_Items(PyObject *o)
- On success, return a list or tuple of the items in object *o*, where each item
- is a tuple containing a key-value pair. On failure, return *NULL*.
+ On success, return a list of the items in object *o*, where each item is a
+ tuple containing a key-value pair. On failure, return *NULL*.
+
+ .. versionchanged:: 3.7
+ Previously, the function returned a list or a tuple.
.. c:function:: PyObject* PyMapping_GetItemString(PyObject *o, const char *key)
diff --git a/Doc/whatsnew/3.7.rst b/Doc/whatsnew/3.7.rst
index ecdd2fe2713..24a1dc4d13b 100644
--- a/Doc/whatsnew/3.7.rst
+++ b/Doc/whatsnew/3.7.rst
@@ -404,6 +404,10 @@ Build and C API Changes
is now of type ``const char *`` rather of ``char *``. (Contributed by Serhiy
Storchaka in :issue:`28769`.)
+* The result of :c:func:`PyMapping_Keys`, :c:func:`PyMapping_Values` and
+ :c:func:`PyMapping_Items` is now always a list, rather than a list or a
+ tuple. (Contributed by Oren Milman in :issue:`28280`.)
+
* Added functions :c:func:`PySlice_Unpack` and :c:func:`PySlice_AdjustIndices`.
(Contributed by Serhiy Storchaka in :issue:`27867`.)
diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py
index 1b826ee8d9a..921735ef5d0 100644
--- a/Lib/test/test_capi.py
+++ b/Lib/test/test_capi.py
@@ -1,7 +1,7 @@
# Run the _testcapi module tests (tests for the Python/C API): by defn,
# these are all functions _testcapi exports whose name begins with 'test_'.
-from collections import namedtuple
+from collections import namedtuple, OrderedDict
import os
import pickle
import platform
@@ -272,6 +272,50 @@ class C(): pass
self.assertIn(b'MemoryError 2 20', out)
self.assertIn(b'MemoryError 3 30', out)
+ def test_mapping_keys_values_items(self):
+ class Mapping1(dict):
+ def keys(self):
+ return list(super().keys())
+ def values(self):
+ return list(super().values())
+ def items(self):
+ return list(super().items())
+ class Mapping2(dict):
+ def keys(self):
+ return tuple(super().keys())
+ def values(self):
+ return tuple(super().values())
+ def items(self):
+ return tuple(super().items())
+ dict_obj = {'foo': 1, 'bar': 2, 'spam': 3}
+
+ for mapping in [{}, OrderedDict(), Mapping1(), Mapping2(),
+ dict_obj, OrderedDict(dict_obj),
+ Mapping1(dict_obj), Mapping2(dict_obj)]:
+ self.assertListEqual(_testcapi.get_mapping_keys(mapping),
+ list(mapping.keys()))
+ self.assertListEqual(_testcapi.get_mapping_values(mapping),
+ list(mapping.values()))
+ self.assertListEqual(_testcapi.get_mapping_items(mapping),
+ list(mapping.items()))
+
+ def test_mapping_keys_values_items_bad_arg(self):
+ self.assertRaises(AttributeError, _testcapi.get_mapping_keys, None)
+ self.assertRaises(AttributeError, _testcapi.get_mapping_values, None)
+ self.assertRaises(AttributeError, _testcapi.get_mapping_items, None)
+
+ class BadMapping:
+ def keys(self):
+ return None
+ def values(self):
+ return None
+ def items(self):
+ return None
+ bad_mapping = BadMapping()
+ self.assertRaises(TypeError, _testcapi.get_mapping_keys, bad_mapping)
+ self.assertRaises(TypeError, _testcapi.get_mapping_values, bad_mapping)
+ self.assertRaises(TypeError, _testcapi.get_mapping_items, bad_mapping)
+
class TestPendingCalls(unittest.TestCase):
diff --git a/Misc/NEWS.d/next/C API/2017-09-30-19-41-44.bpo-28280.K_EjpO.rst b/Misc/NEWS.d/next/C API/2017-09-30-19-41-44.bpo-28280.K_EjpO.rst
new file mode 100644
index 00000000000..76990f79cb0
--- /dev/null
+++ b/Misc/NEWS.d/next/C API/2017-09-30-19-41-44.bpo-28280.K_EjpO.rst
@@ -0,0 +1,2 @@
+Make `PyMapping_Keys()`, `PyMapping_Values()` and `PyMapping_Items()` always
+return a `list` (rather than a `list` or a `tuple`). Patch by Oren Milman.
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index b512c05a3fe..6e31105712f 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -4307,6 +4307,25 @@ py_w_stopcode(PyObject *self, PyObject *args)
static PyObject *
+get_mapping_keys(PyObject* self, PyObject *obj)
+{
+ return PyMapping_Keys(obj);
+}
+
+static PyObject *
+get_mapping_values(PyObject* self, PyObject *obj)
+{
+ return PyMapping_Values(obj);
+}
+
+static PyObject *
+get_mapping_items(PyObject* self, PyObject *obj)
+{
+ return PyMapping_Items(obj);
+}
+
+
+static PyObject *
test_pythread_tss_key_state(PyObject *self, PyObject *args)
{
Py_tss_t tss_key = Py_tss_NEEDS_INIT;
@@ -4573,6 +4592,9 @@ static PyMethodDef TestMethods[] = {
#ifdef W_STOPCODE
{"W_STOPCODE", py_w_stopcode, METH_VARARGS},
#endif
+ {"get_mapping_keys", get_mapping_keys, METH_O},
+ {"get_mapping_values", get_mapping_values, METH_O},
+ {"get_mapping_items", get_mapping_items, METH_O},
{"test_pythread_tss_key_state", test_pythread_tss_key_state, METH_VARARGS},
{NULL, NULL} /* sentinel */
};
diff --git a/Objects/abstract.c b/Objects/abstract.c
index 38484b79a1b..3cb7a32b01e 100644
--- a/Objects/abstract.c
+++ b/Objects/abstract.c
@@ -2147,55 +2147,77 @@ PyMapping_HasKey(PyObject *o, PyObject *key)
return 0;
}
+/* This function is quite similar to PySequence_Fast(), but specialized to be
+ a helper for PyMapping_Keys(), PyMapping_Items() and PyMapping_Values().
+ */
+static PyObject *
+method_output_as_list(PyObject *o, _Py_Identifier *meth_id)
+{
+ PyObject *it, *result, *meth_output;
+
+ assert(o != NULL);
+ meth_output = _PyObject_CallMethodId(o, meth_id, NULL);
+ if (meth_output == NULL || PyList_CheckExact(meth_output)) {
+ return meth_output;
+ }
+ it = PyObject_GetIter(meth_output);
+ if (it == NULL) {
+ if (PyErr_ExceptionMatches(PyExc_TypeError)) {
+ PyErr_Format(PyExc_TypeError,
+ "%.200s.%U() returned a non-iterable (type %.200s)",
+ Py_TYPE(o)->tp_name,
+ meth_id->object,
+ Py_TYPE(meth_output)->tp_name);
+ }
+ Py_DECREF(meth_output);
+ return NULL;
+ }
+ Py_DECREF(meth_output);
+ result = PySequence_List(it);
+ Py_DECREF(it);
+ return result;
+}
+
PyObject *
PyMapping_Keys(PyObject *o)
{
- PyObject *keys;
- PyObject *fast;
_Py_IDENTIFIER(keys);
- if (PyDict_CheckExact(o))
+ if (o == NULL) {
+ return null_error();
+ }
+ if (PyDict_CheckExact(o)) {
return PyDict_Keys(o);
- keys = _PyObject_CallMethodId(o, &PyId_keys, NULL);
- if (keys == NULL)
- return NULL;
- fast = PySequence_Fast(keys, "o.keys() are not iterable");
- Py_DECREF(keys);
- return fast;
+ }
+ return method_output_as_list(o, &PyId_keys);
}
PyObject *
PyMapping_Items(PyObject *o)
{
- PyObject *items;
- PyObject *fast;
_Py_IDENTIFIER(items);
- if (PyDict_CheckExact(o))
+ if (o == NULL) {
+ return null_error();
+ }
+ if (PyDict_CheckExact(o)) {
return PyDict_Items(o);
- items = _PyObject_CallMethodId(o, &PyId_items, NULL);
- if (items == NULL)
- return NULL;
- fast = PySequence_Fast(items, "o.items() are not iterable");
- Py_DECREF(items);
- return fast;
+ }
+ return method_output_as_list(o, &PyId_items);
}
PyObject *
PyMapping_Values(PyObject *o)
{
- PyObject *values;
- PyObject *fast;
_Py_IDENTIFIER(values);
- if (PyDict_CheckExact(o))
+ if (o == NULL) {
+ return null_error();
+ }
+ if (PyDict_CheckExact(o)) {
return PyDict_Values(o);
- values = _PyObject_CallMethodId(o, &PyId_values, NULL);
- if (values == NULL)
- return NULL;
- fast = PySequence_Fast(values, "o.values() are not iterable");
- Py_DECREF(values);
- return fast;
+ }
+ return method_output_as_list(o, &PyId_values);
}
/* isinstance(), issubclass() */
More information about the Python-checkins
mailing list