Skip to content

Commit e036ef8

Browse files
serhiy-storchakaserhiy-storchaka
committed
Issue #27358: Optimized merging var-keyword arguments and improved error
message when pass a non-mapping as a var-keyword argument.
1 parent 0a3beff commit e036ef8

File tree

5 files changed

+124
-53
lines changed

5 files changed

+124
-53
lines changed

‎Include/dictobject.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,12 @@ PyAPI_FUNC(int) PyDict_Merge(PyObject *mp,
132132
int override);
133133

134134
#ifndef Py_LIMITED_API
135+
/* Like PyDict_Merge, but override can be 0, 1 or 2. If override is 0,
136+
the first occurrence of a key wins, if override is 1, the last occurrence
137+
of a key wins, if override is 2, a KeyError with conflicting key as
138+
argument is raised.
139+
*/
140+
PyAPI_FUNC(int) _PyDict_MergeEx(PyObject *mp, PyObject *other, int override);
135141
PyAPI_FUNC(PyObject *) _PyDictView_Intersect(PyObject* self, PyObject *other);
136142
#endif
137143

‎Lib/test/test_extcall.py‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,31 @@
259259
...
260260
TypeError: h() argument after ** must be a mapping, not function
261261
262+
>>> h(**[])
263+
Traceback (most recent call last):
264+
...
265+
TypeError: h() argument after ** must be a mapping, not list
266+
267+
>>> h(a=1, **h)
268+
Traceback (most recent call last):
269+
...
270+
TypeError: h() argument after ** must be a mapping, not function
271+
272+
>>> h(a=1, **[])
273+
Traceback (most recent call last):
274+
...
275+
TypeError: h() argument after ** must be a mapping, not list
276+
277+
>>> h(**{'a': 1}, **h)
278+
Traceback (most recent call last):
279+
...
280+
TypeError: h() argument after ** must be a mapping, not function
281+
282+
>>> h(**{'a': 1}, **[])
283+
Traceback (most recent call last):
284+
...
285+
TypeError: h() argument after ** must be a mapping, not list
286+
262287
>>> dir(**h)
263288
Traceback (most recent call last):
264289
...

‎Misc/NEWS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ Core and Builtins
4646
Library
4747
-------
4848

49+
- Issue #27358: Optimized merging var-keyword arguments and improved error
50+
message when pass a non-mapping as a var-keyword argument.
51+
4952
- Issue #28257: Improved error message when pass a non-iterable as
5053
a var-positional argument. Added opcode BUILD_TUPLE_UNPACK_WITH_CALL.
5154

‎Objects/dictobject.c‎

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2380,18 +2380,14 @@ PyDict_MergeFromSeq2(PyObject *d, PyObject *seq2, int override)
23802380
}
23812381

23822382
int
2383-
PyDict_Update(PyObject *a, PyObject *b)
2384-
{
2385-
return PyDict_Merge(a, b, 1);
2386-
}
2387-
2388-
int
2389-
PyDict_Merge(PyObject *a, PyObject *b, int override)
2383+
dict_merge(PyObject *a, PyObject *b, int override)
23902384
{
23912385
PyDictObject *mp, *other;
23922386
Py_ssize_t i, n;
23932387
PyDictKeyEntry *entry, *ep0;
23942388

2389+
assert(0 <= override && override <= 2);
2390+
23952391
/* We accept for the argument either a concrete dictionary object,
23962392
* or an abstract "mapping" object. For the former, we can do
23972393
* things quite efficiently. For the latter, we only require that
@@ -2436,8 +2432,14 @@ PyDict_Merge(PyObject *a, PyObject *b, int override)
24362432
int err = 0;
24372433
Py_INCREF(key);
24382434
Py_INCREF(value);
2439-
if (override || PyDict_GetItem(a, key) == NULL)
2435+
if (override == 1 || _PyDict_GetItem_KnownHash(a, key, hash) == NULL)
24402436
err = insertdict(mp, key, hash, value);
2437+
else if (override != 0) {
2438+
_PyErr_SetKeyError(key);
2439+
Py_DECREF(value);
2440+
Py_DECREF(key);
2441+
return -1;
2442+
}
24412443
Py_DECREF(value);
24422444
Py_DECREF(key);
24432445
if (err != 0)
@@ -2472,7 +2474,13 @@ PyDict_Merge(PyObject *a, PyObject *b, int override)
24722474
return -1;
24732475

24742476
for (key = PyIter_Next(iter); key; key = PyIter_Next(iter)) {
2475-
if (!override && PyDict_GetItem(a, key) != NULL) {
2477+
if (override != 1 && PyDict_GetItem(a, key) != NULL) {
2478+
if (override != 0) {
2479+
_PyErr_SetKeyError(key);
2480+
Py_DECREF(key);
2481+
Py_DECREF(iter);
2482+
return -1;
2483+
}
24762484
Py_DECREF(key);
24772485
continue;
24782486
}
@@ -2499,6 +2507,25 @@ PyDict_Merge(PyObject *a, PyObject *b, int override)
24992507
return 0;
25002508
}
25012509

2510+
int
2511+
PyDict_Update(PyObject *a, PyObject *b)
2512+
{
2513+
return dict_merge(a, b, 1);
2514+
}
2515+
2516+
int
2517+
PyDict_Merge(PyObject *a, PyObject *b, int override)
2518+
{
2519+
/* XXX Deprecate override not in (0, 1). */
2520+
return dict_merge(a, b, override != 0);
2521+
}
2522+
2523+
int
2524+
_PyDict_MergeEx(PyObject *a, PyObject *b, int override)
2525+
{
2526+
return dict_merge(a, b, override);
2527+
}
2528+
25022529
static PyObject *
25032530
dict_copy(PyDictObject *mp)
25042531
{

‎Python/ceval.c‎

Lines changed: 54 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2710,65 +2710,75 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
27102710
DISPATCH();
27112711
}
27122712

2713-
TARGET(BUILD_MAP_UNPACK_WITH_CALL)
27142713
TARGET(BUILD_MAP_UNPACK) {
2715-
int with_call = opcode == BUILD_MAP_UNPACK_WITH_CALL;
27162714
Py_ssize_t i;
27172715
PyObject *sum = PyDict_New();
27182716
if (sum == NULL)
27192717
goto error;
27202718

27212719
for (i = oparg; i > 0; i--) {
27222720
PyObject *arg = PEEK(i);
2723-
if (with_call && PyDict_Size(sum)) {
2724-
PyObject *intersection = _PyDictView_Intersect(sum, arg);
2725-
2726-
if (intersection == NULL) {
2727-
if (PyErr_ExceptionMatches(PyExc_AttributeError)) {
2728-
PyObject *func = PEEK(2 + oparg);
2729-
PyErr_Format(PyExc_TypeError,
2730-
"%.200s%.200s argument after ** "
2731-
"must be a mapping, not %.200s",
2732-
PyEval_GetFuncName(func),
2733-
PyEval_GetFuncDesc(func),
2734-
arg->ob_type->tp_name);
2735-
}
2736-
Py_DECREF(sum);
2737-
goto error;
2738-
}
2739-
2740-
if (PySet_GET_SIZE(intersection)) {
2741-
Py_ssize_t idx = 0;
2742-
PyObject *key;
2743-
PyObject *func = PEEK(2 + oparg);
2744-
Py_hash_t hash;
2745-
_PySet_NextEntry(intersection, &idx, &key, &hash);
2746-
if (!PyUnicode_Check(key)) {
2747-
PyErr_Format(PyExc_TypeError,
2748-
"%.200s%.200s keywords must be strings",
2749-
PyEval_GetFuncName(func),
2750-
PyEval_GetFuncDesc(func));
2751-
} else {
2752-
PyErr_Format(PyExc_TypeError,
2753-
"%.200s%.200s got multiple "
2754-
"values for keyword argument '%U'",
2755-
PyEval_GetFuncName(func),
2756-
PyEval_GetFuncDesc(func),
2757-
key);
2758-
}
2759-
Py_DECREF(intersection);
2760-
Py_DECREF(sum);
2761-
goto error;
2721+
if (PyDict_Update(sum, arg) < 0) {
2722+
if (PyErr_ExceptionMatches(PyExc_AttributeError)) {
2723+
PyErr_Format(PyExc_TypeError,
2724+
"'%.200s' object is not a mapping1",
2725+
arg->ob_type->tp_name);
27622726
}
2763-
Py_DECREF(intersection);
2727+
Py_DECREF(sum);
2728+
goto error;
27642729
}
2730+
}
27652731

2766-
if (PyDict_Update(sum, arg) < 0) {
2732+
while (oparg--)
2733+
Py_DECREF(POP());
2734+
PUSH(sum);
2735+
DISPATCH();
2736+
}
2737+
2738+
TARGET(BUILD_MAP_UNPACK_WITH_CALL) {
2739+
Py_ssize_t i;
2740+
PyObject *sum = PyDict_New();
2741+
if (sum == NULL)
2742+
goto error;
2743+
2744+
for (i = oparg; i > 0; i--) {
2745+
PyObject *arg = PEEK(i);
2746+
if (_PyDict_MergeEx(sum, arg, 2) < 0) {
2747+
PyObject *func = PEEK(2 + oparg);
27672748
if (PyErr_ExceptionMatches(PyExc_AttributeError)) {
27682749
PyErr_Format(PyExc_TypeError,
2769-
"'%.200s' object is not a mapping",
2750+
"%.200s%.200s argument after ** "
2751+
"must be a mapping, not %.200s",
2752+
PyEval_GetFuncName(func),
2753+
PyEval_GetFuncDesc(func),
27702754
arg->ob_type->tp_name);
27712755
}
2756+
else if (PyErr_ExceptionMatches(PyExc_KeyError)) {
2757+
PyObject *exc, *val, *tb;
2758+
PyErr_Fetch(&exc, &val, &tb);
2759+
if (val && PyTuple_Check(val) && PyTuple_GET_SIZE(val) == 1) {
2760+
PyObject *key = PyTuple_GET_ITEM(val, 0);
2761+
if (!PyUnicode_Check(key)) {
2762+
PyErr_Format(PyExc_TypeError,
2763+
"%.200s%.200s keywords must be strings",
2764+
PyEval_GetFuncName(func),
2765+
PyEval_GetFuncDesc(func));
2766+
} else {
2767+
PyErr_Format(PyExc_TypeError,
2768+
"%.200s%.200s got multiple "
2769+
"values for keyword argument '%U'",
2770+
PyEval_GetFuncName(func),
2771+
PyEval_GetFuncDesc(func),
2772+
key);
2773+
}
2774+
Py_XDECREF(exc);
2775+
Py_XDECREF(val);
2776+
Py_XDECREF(tb);
2777+
}
2778+
else {
2779+
PyErr_Restore(exc, val, tb);
2780+
}
2781+
}
27722782
Py_DECREF(sum);
27732783
goto error;
27742784
}

0 commit comments

Comments
 (0)