Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 0ccc0f6

Browse filesBrowse files
orenmnserhiy-storchaka
orenmn
authored and
serhiy-storchaka
committed
bpo-28280: Make PyMapping_Keys(), PyMapping_Values() and PyMapping_Items() always return a list (#3840)
1 parent f07e2b6 commit 0ccc0f6
Copy full SHA for 0ccc0f6

File tree

6 files changed

+137
-34
lines changed
Filter options

6 files changed

+137
-34
lines changed

‎Doc/c-api/mapping.rst

Copy file name to clipboardExpand all lines: Doc/c-api/mapping.rst
+15-6Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,29 @@ Mapping Protocol
5050
5151
.. c:function:: PyObject* PyMapping_Keys(PyObject *o)
5252
53-
On success, return a list or tuple of the keys in object *o*. On failure,
54-
return *NULL*.
53+
On success, return a list of the keys in object *o*. On failure, return
54+
*NULL*.
55+
56+
.. versionchanged:: 3.7
57+
Previously, the function returned a list or a tuple.
5558
5659
5760
.. c:function:: PyObject* PyMapping_Values(PyObject *o)
5861
59-
On success, return a list or tuple of the values in object *o*. On failure,
60-
return *NULL*.
62+
On success, return a list of the values in object *o*. On failure, return
63+
*NULL*.
64+
65+
.. versionchanged:: 3.7
66+
Previously, the function returned a list or a tuple.
6167
6268
6369
.. c:function:: PyObject* PyMapping_Items(PyObject *o)
6470
65-
On success, return a list or tuple of the items in object *o*, where each item
66-
is a tuple containing a key-value pair. On failure, return *NULL*.
71+
On success, return a list of the items in object *o*, where each item is a
72+
tuple containing a key-value pair. On failure, return *NULL*.
73+
74+
.. versionchanged:: 3.7
75+
Previously, the function returned a list or a tuple.
6776
6877
6978
.. c:function:: PyObject* PyMapping_GetItemString(PyObject *o, const char *key)

‎Doc/whatsnew/3.7.rst

Copy file name to clipboardExpand all lines: Doc/whatsnew/3.7.rst
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,10 @@ Build and C API Changes
404404
is now of type ``const char *`` rather of ``char *``. (Contributed by Serhiy
405405
Storchaka in :issue:`28769`.)
406406

407+
* The result of :c:func:`PyMapping_Keys`, :c:func:`PyMapping_Values` and
408+
:c:func:`PyMapping_Items` is now always a list, rather than a list or a
409+
tuple. (Contributed by Oren Milman in :issue:`28280`.)
410+
407411
* Added functions :c:func:`PySlice_Unpack` and :c:func:`PySlice_AdjustIndices`.
408412
(Contributed by Serhiy Storchaka in :issue:`27867`.)
409413

‎Lib/test/test_capi.py

Copy file name to clipboardExpand all lines: Lib/test/test_capi.py
+45-1Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Run the _testcapi module tests (tests for the Python/C API): by defn,
22
# these are all functions _testcapi exports whose name begins with 'test_'.
33

4-
from collections import namedtuple
4+
from collections import namedtuple, OrderedDict
55
import os
66
import pickle
77
import platform
@@ -272,6 +272,50 @@ class C(): pass
272272
self.assertIn(b'MemoryError 2 20', out)
273273
self.assertIn(b'MemoryError 3 30', out)
274274

275+
def test_mapping_keys_values_items(self):
276+
class Mapping1(dict):
277+
def keys(self):
278+
return list(super().keys())
279+
def values(self):
280+
return list(super().values())
281+
def items(self):
282+
return list(super().items())
283+
class Mapping2(dict):
284+
def keys(self):
285+
return tuple(super().keys())
286+
def values(self):
287+
return tuple(super().values())
288+
def items(self):
289+
return tuple(super().items())
290+
dict_obj = {'foo': 1, 'bar': 2, 'spam': 3}
291+
292+
for mapping in [{}, OrderedDict(), Mapping1(), Mapping2(),
293+
dict_obj, OrderedDict(dict_obj),
294+
Mapping1(dict_obj), Mapping2(dict_obj)]:
295+
self.assertListEqual(_testcapi.get_mapping_keys(mapping),
296+
list(mapping.keys()))
297+
self.assertListEqual(_testcapi.get_mapping_values(mapping),
298+
list(mapping.values()))
299+
self.assertListEqual(_testcapi.get_mapping_items(mapping),
300+
list(mapping.items()))
301+
302+
def test_mapping_keys_values_items_bad_arg(self):
303+
self.assertRaises(AttributeError, _testcapi.get_mapping_keys, None)
304+
self.assertRaises(AttributeError, _testcapi.get_mapping_values, None)
305+
self.assertRaises(AttributeError, _testcapi.get_mapping_items, None)
306+
307+
class BadMapping:
308+
def keys(self):
309+
return None
310+
def values(self):
311+
return None
312+
def items(self):
313+
return None
314+
bad_mapping = BadMapping()
315+
self.assertRaises(TypeError, _testcapi.get_mapping_keys, bad_mapping)
316+
self.assertRaises(TypeError, _testcapi.get_mapping_values, bad_mapping)
317+
self.assertRaises(TypeError, _testcapi.get_mapping_items, bad_mapping)
318+
275319

276320
class TestPendingCalls(unittest.TestCase):
277321

‎Misc/NEWS.d/next/C API/2017-09-30-19-41-44.bpo-28280.K_EjpO.rst

Copy file name to clipboard
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Make `PyMapping_Keys()`, `PyMapping_Values()` and `PyMapping_Items()` always
2+
return a `list` (rather than a `list` or a `tuple`). Patch by Oren Milman.

‎Modules/_testcapimodule.c

Copy file name to clipboardExpand all lines: Modules/_testcapimodule.c
+22Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4306,6 +4306,25 @@ py_w_stopcode(PyObject *self, PyObject *args)
43064306
#endif
43074307

43084308

4309+
static PyObject *
4310+
get_mapping_keys(PyObject* self, PyObject *obj)
4311+
{
4312+
return PyMapping_Keys(obj);
4313+
}
4314+
4315+
static PyObject *
4316+
get_mapping_values(PyObject* self, PyObject *obj)
4317+
{
4318+
return PyMapping_Values(obj);
4319+
}
4320+
4321+
static PyObject *
4322+
get_mapping_items(PyObject* self, PyObject *obj)
4323+
{
4324+
return PyMapping_Items(obj);
4325+
}
4326+
4327+
43094328
static PyObject *
43104329
test_pythread_tss_key_state(PyObject *self, PyObject *args)
43114330
{
@@ -4573,6 +4592,9 @@ static PyMethodDef TestMethods[] = {
45734592
#ifdef W_STOPCODE
45744593
{"W_STOPCODE", py_w_stopcode, METH_VARARGS},
45754594
#endif
4595+
{"get_mapping_keys", get_mapping_keys, METH_O},
4596+
{"get_mapping_values", get_mapping_values, METH_O},
4597+
{"get_mapping_items", get_mapping_items, METH_O},
45764598
{"test_pythread_tss_key_state", test_pythread_tss_key_state, METH_VARARGS},
45774599
{NULL, NULL} /* sentinel */
45784600
};

‎Objects/abstract.c

Copy file name to clipboardExpand all lines: Objects/abstract.c
+49-27Lines changed: 49 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2147,55 +2147,77 @@ PyMapping_HasKey(PyObject *o, PyObject *key)
21472147
return 0;
21482148
}
21492149

2150+
/* This function is quite similar to PySequence_Fast(), but specialized to be
2151+
a helper for PyMapping_Keys(), PyMapping_Items() and PyMapping_Values().
2152+
*/
2153+
static PyObject *
2154+
method_output_as_list(PyObject *o, _Py_Identifier *meth_id)
2155+
{
2156+
PyObject *it, *result, *meth_output;
2157+
2158+
assert(o != NULL);
2159+
meth_output = _PyObject_CallMethodId(o, meth_id, NULL);
2160+
if (meth_output == NULL || PyList_CheckExact(meth_output)) {
2161+
return meth_output;
2162+
}
2163+
it = PyObject_GetIter(meth_output);
2164+
if (it == NULL) {
2165+
if (PyErr_ExceptionMatches(PyExc_TypeError)) {
2166+
PyErr_Format(PyExc_TypeError,
2167+
"%.200s.%U() returned a non-iterable (type %.200s)",
2168+
Py_TYPE(o)->tp_name,
2169+
meth_id->object,
2170+
Py_TYPE(meth_output)->tp_name);
2171+
}
2172+
Py_DECREF(meth_output);
2173+
return NULL;
2174+
}
2175+
Py_DECREF(meth_output);
2176+
result = PySequence_List(it);
2177+
Py_DECREF(it);
2178+
return result;
2179+
}
2180+
21502181
PyObject *
21512182
PyMapping_Keys(PyObject *o)
21522183
{
2153-
PyObject *keys;
2154-
PyObject *fast;
21552184
_Py_IDENTIFIER(keys);
21562185

2157-
if (PyDict_CheckExact(o))
2186+
if (o == NULL) {
2187+
return null_error();
2188+
}
2189+
if (PyDict_CheckExact(o)) {
21582190
return PyDict_Keys(o);
2159-
keys = _PyObject_CallMethodId(o, &PyId_keys, NULL);
2160-
if (keys == NULL)
2161-
return NULL;
2162-
fast = PySequence_Fast(keys, "o.keys() are not iterable");
2163-
Py_DECREF(keys);
2164-
return fast;
2191+
}
2192+
return method_output_as_list(o, &PyId_keys);
21652193
}
21662194

21672195
PyObject *
21682196
PyMapping_Items(PyObject *o)
21692197
{
2170-
PyObject *items;
2171-
PyObject *fast;
21722198
_Py_IDENTIFIER(items);
21732199

2174-
if (PyDict_CheckExact(o))
2200+
if (o == NULL) {
2201+
return null_error();
2202+
}
2203+
if (PyDict_CheckExact(o)) {
21752204
return PyDict_Items(o);
2176-
items = _PyObject_CallMethodId(o, &PyId_items, NULL);
2177-
if (items == NULL)
2178-
return NULL;
2179-
fast = PySequence_Fast(items, "o.items() are not iterable");
2180-
Py_DECREF(items);
2181-
return fast;
2205+
}
2206+
return method_output_as_list(o, &PyId_items);
21822207
}
21832208

21842209
PyObject *
21852210
PyMapping_Values(PyObject *o)
21862211
{
2187-
PyObject *values;
2188-
PyObject *fast;
21892212
_Py_IDENTIFIER(values);
21902213

2191-
if (PyDict_CheckExact(o))
2214+
if (o == NULL) {
2215+
return null_error();
2216+
}
2217+
if (PyDict_CheckExact(o)) {
21922218
return PyDict_Values(o);
2193-
values = _PyObject_CallMethodId(o, &PyId_values, NULL);
2194-
if (values == NULL)
2195-
return NULL;
2196-
fast = PySequence_Fast(values, "o.values() are not iterable");
2197-
Py_DECREF(values);
2198-
return fast;
2219+
}
2220+
return method_output_as_list(o, &PyId_values);
21992221
}
22002222

22012223
/* isinstance(), issubclass() */

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.