From 82837ae96eb676338b1ce14efad7ad2b257e1bbc Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Wed, 4 Nov 2026 22:29:54 -0800 Subject: [PATCH] gh-87613: Pass NULL for an empty **kwargs in Argument Clinic The parser allocated an empty dict for **kwargs when there were no keyword arguments. Pass NULL instead, like the caller does, and handle NULL in the impls that used the dict unconditionally. All **kwargs impls were audited: _csv and zoneinfo already accept NULL, and the rest (structseq and SimpleNamespace `__replace__`, _interpreters.new_config, atexit.register, SubElement) now check for it. atexit stores None, which the run loop already maps back to NULL. pyperf, PGO+LTO, `t = time.gmtime(0)`, `ns = SimpleNamespace(a=1)`: | Benchmark | main | this change | |---------------------------------|:------:|:---------------------:| | struct_time.`__replace__`() | 229 ns | 47.0 ns: 4.87x faster | | SimpleNamespace.`__replace__`() | 72.2 ns | 65.5 ns: 1.10x faster | Co-Authored-By: Claude Opus 5.5 --- Lib/test/test_clinic.py | 14 +++---- Modules/_elementtree.c | 4 +- Modules/_interpretersmodule.c | 2 +- Modules/atexitmodule.c | 3 +- Modules/clinic/_csv.c.h | 42 +++----------------- Modules/clinic/_elementtree.c.h | 12 +----- Modules/clinic/_interpretersmodule.c.h | 12 +----- Modules/clinic/_testclinic_kwds.c.h | 53 +++----------------------- Modules/clinic/_zoneinfo.c.h | 12 +----- Modules/clinic/atexitmodule.c.h | 12 +----- Objects/clinic/namespaceobject.c.h | 12 +----- Objects/clinic/structseq.c.h | 12 +----- Objects/namespaceobject.c | 4 +- Objects/structseq.c | 9 +++-- Tools/clinic/libclinic/converters.py | 14 +------ 15 files changed, 45 insertions(+), 172 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index e57f4c4a9d556af..4e0b0137e9faa0b 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -5708,7 +5708,7 @@ def errmsg(name): def test_lone_kwds(self): with self.assertRaises(TypeError): ac_tester.lone_kwds(1, 2) - self.assertEqual(ac_tester.lone_kwds(), ({},)) + self.assertEqual(ac_tester.lone_kwds(), (None,)) self.assertEqual(ac_tester.lone_kwds(y='y'), ({'y': 'y'},)) kwds = {'y': 'y', 'z': 'z'} self.assertEqual(ac_tester.lone_kwds(y='y', z='z'), (kwds,)) @@ -5721,7 +5721,7 @@ def test_kwds_with_pos_only(self): ac_tester.kwds_with_pos_only(y='y') with self.assertRaises(TypeError): ac_tester.kwds_with_pos_only(1, y='y') - self.assertEqual(ac_tester.kwds_with_pos_only(1, 2), (1, 2, {})) + self.assertEqual(ac_tester.kwds_with_pos_only(1, 2), (1, 2, None)) self.assertEqual(ac_tester.kwds_with_pos_only(1, 2, y='y'), (1, 2, {'y': 'y'})) kwds = {'y': 'y', 'z': 'z'} self.assertEqual(ac_tester.kwds_with_pos_only(1, 2, y='y', z='z'), (1, 2, kwds)) @@ -5732,16 +5732,16 @@ def test_kwds_with_optional_pos_only(self): ac_tester.kwds_with_optional_pos_only() with self.assertRaises(TypeError): ac_tester.kwds_with_optional_pos_only(y='y') - self.assertEqual(ac_tester.kwds_with_optional_pos_only(1), (1, None, {})) - self.assertEqual(ac_tester.kwds_with_optional_pos_only(1, 2), (1, 2, {})) + self.assertEqual(ac_tester.kwds_with_optional_pos_only(1), (1, None, None)) + self.assertEqual(ac_tester.kwds_with_optional_pos_only(1, 2), (1, 2, None)) self.assertEqual(ac_tester.kwds_with_optional_pos_only(1, y='y'), (1, None, {'y': 'y'})) self.assertEqual(ac_tester.kwds_with_optional_pos_only(1, 2, y='y'), (1, 2, {'y': 'y'})) def test_kwds_with_stararg(self): - self.assertEqual(ac_tester.kwds_with_stararg(), ((), {})) - self.assertEqual(ac_tester.kwds_with_stararg(1, 2), ((1, 2), {})) + self.assertEqual(ac_tester.kwds_with_stararg(), ((), None)) + self.assertEqual(ac_tester.kwds_with_stararg(1, 2), ((1, 2), None)) self.assertEqual(ac_tester.kwds_with_stararg(y='y'), ((), {'y': 'y'})) args = (1, 2) kwds = {'y': 'y', 'z': 'z'} @@ -5753,7 +5753,7 @@ def test_kwds_with_pos_only_and_stararg(self): ac_tester.kwds_with_pos_only_and_stararg() with self.assertRaises(TypeError): ac_tester.kwds_with_pos_only_and_stararg(y='y') - self.assertEqual(ac_tester.kwds_with_pos_only_and_stararg(1, 2), (1, 2, (), {})) + self.assertEqual(ac_tester.kwds_with_pos_only_and_stararg(1, 2), (1, 2, (), None)) self.assertEqual(ac_tester.kwds_with_pos_only_and_stararg(1, 2, y='y'), (1, 2, (), {'y': 'y'})) args = ('lobster', 'thermidor') kwds = {'y': 'y', 'z': 'z'} diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c index f8f661a73418912..983a00593b85d89 100644 --- a/Modules/_elementtree.c +++ b/Modules/_elementtree.c @@ -640,11 +640,11 @@ _elementtree_SubElement_impl(PyObject *module, PyObject *parent, attrib = PyDict_Copy(attrib); if (!attrib) return NULL; - if (PyDict_Update(attrib, extra) < 0) { + if (extra && PyDict_Update(attrib, extra) < 0) { Py_DECREF(attrib); return NULL; } - } else if (PyDict_GET_SIZE(extra)) { + } else if (extra && PyDict_GET_SIZE(extra)) { /* have keyword args */ attrib = get_attrib_from_keywords(extra); if (!attrib) diff --git a/Modules/_interpretersmodule.c b/Modules/_interpretersmodule.c index 6630d5f6fe2d33c..454db063d18b6a0 100644 --- a/Modules/_interpretersmodule.c +++ b/Modules/_interpretersmodule.c @@ -828,7 +828,7 @@ _interpreters_new_config_impl(PyObject *module, const char *name, return NULL; } - if (PyDict_GET_SIZE(overrides) > 0) { + if (overrides != NULL && PyDict_GET_SIZE(overrides) > 0) { if (_PyInterpreterConfig_UpdateFromDict(&config, overrides) < 0) { return NULL; } diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index b835fe670938cbc..d572351e80f6586 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -191,7 +191,8 @@ atexit_register_impl(PyObject *module, PyObject *func, PyObject *args, "the first argument must be callable"); return NULL; } - PyObject *callback = PyTuple_Pack(3, func, args, kwargs); + PyObject *callback = PyTuple_Pack(3, func, args, + kwargs ? kwargs : Py_None); if (callback == NULL) { return NULL; diff --git a/Modules/clinic/_csv.c.h b/Modules/clinic/_csv.c.h index df3bdbadf29aed6..09dbcbea7e057b1 100644 --- a/Modules/clinic/_csv.c.h +++ b/Modules/clinic/_csv.c.h @@ -70,15 +70,7 @@ _csv_Dialect___replace__(PyObject *self, PyObject *args, PyObject *kwargs) if (!_PyArg_NoPositional("__replace__", args)) { goto exit; } - if (kwargs == NULL) { - changes = PyDict_New(); - if (changes == NULL) { - goto exit; - } - } - else { - changes = Py_NewRef(kwargs); - } + changes = Py_XNewRef(kwargs); return_value = _csv_Dialect___replace___impl((DialectObj *)self, changes); exit: @@ -142,15 +134,7 @@ _csv_reader(PyObject *module, PyObject *args, PyObject *kwargs) } dialect = PyTuple_GET_ITEM(args, 1); skip_optional: - if (kwargs == NULL) { - fmtparams = PyDict_New(); - if (fmtparams == NULL) { - goto exit; - } - } - else { - fmtparams = Py_NewRef(kwargs); - } + fmtparams = Py_XNewRef(kwargs); return_value = _csv_reader_impl(module, iterable, dialect, fmtparams); exit: @@ -195,15 +179,7 @@ _csv_writer(PyObject *module, PyObject *args, PyObject *kwargs) } dialect = PyTuple_GET_ITEM(args, 1); skip_optional: - if (kwargs == NULL) { - fmtparams = PyDict_New(); - if (fmtparams == NULL) { - goto exit; - } - } - else { - fmtparams = Py_NewRef(kwargs); - } + fmtparams = Py_XNewRef(kwargs); return_value = _csv_writer_impl(module, output_file, dialect, fmtparams); exit: @@ -266,15 +242,7 @@ _csv_register_dialect(PyObject *module, PyObject *args, PyObject *kwargs) } dialect_obj = PyTuple_GET_ITEM(args, 1); skip_optional: - if (kwargs == NULL) { - fmtparams = PyDict_New(); - if (fmtparams == NULL) { - goto exit; - } - } - else { - fmtparams = Py_NewRef(kwargs); - } + fmtparams = Py_XNewRef(kwargs); return_value = _csv_register_dialect_impl(module, name_obj, dialect_obj, fmtparams); exit: @@ -465,4 +433,4 @@ _csv_field_size_limit(PyObject *module, PyObject *const *args, Py_ssize_t nargs, exit: return return_value; } -/*[clinic end generated code: output=7e3a92d524d88023 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=30eac1c04ff9d38a input=a9049054013a1b77]*/ diff --git a/Modules/clinic/_elementtree.c.h b/Modules/clinic/_elementtree.c.h index 9b0494f35fae9a8..afc9daa5288f2c5 100644 --- a/Modules/clinic/_elementtree.c.h +++ b/Modules/clinic/_elementtree.c.h @@ -51,15 +51,7 @@ _elementtree_SubElement(PyObject *module, PyObject *args, PyObject *kwargs) } attrib = PyTuple_GET_ITEM(args, 2); skip_optional: - if (kwargs == NULL) { - extra = PyDict_New(); - if (extra == NULL) { - goto exit; - } - } - else { - extra = Py_NewRef(kwargs); - } + extra = Py_XNewRef(kwargs); return_value = _elementtree_SubElement_impl(module, parent, tag, attrib, extra); exit: @@ -1482,4 +1474,4 @@ _elementtree_XMLParser__setevents(PyObject *self, PyObject *const *args, Py_ssiz exit: return return_value; } -/*[clinic end generated code: output=01693c4bcc6c63b2 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=d454d27e90eb399e input=a9049054013a1b77]*/ diff --git a/Modules/clinic/_interpretersmodule.c.h b/Modules/clinic/_interpretersmodule.c.h index f297280279c5cdf..4d087b152d2b7f7 100644 --- a/Modules/clinic/_interpretersmodule.c.h +++ b/Modules/clinic/_interpretersmodule.c.h @@ -50,15 +50,7 @@ _interpreters_new_config(PyObject *module, PyObject *args, PyObject *kwargs) goto exit; } skip_optional: - if (kwargs == NULL) { - overrides = PyDict_New(); - if (overrides == NULL) { - goto exit; - } - } - else { - overrides = Py_NewRef(kwargs); - } + overrides = Py_XNewRef(kwargs); return_value = _interpreters_new_config_impl(module, name, overrides); exit: @@ -1260,4 +1252,4 @@ _interpreters_capture_exception(PyObject *module, PyObject *const *args, Py_ssiz exit: return return_value; } -/*[clinic end generated code: output=fa37872d6ee729ef input=a9049054013a1b77]*/ +/*[clinic end generated code: output=ad79d44c2b139a83 input=a9049054013a1b77]*/ diff --git a/Modules/clinic/_testclinic_kwds.c.h b/Modules/clinic/_testclinic_kwds.c.h index faae3f3d0c26584..729ba1b6fa7c3da 100644 --- a/Modules/clinic/_testclinic_kwds.c.h +++ b/Modules/clinic/_testclinic_kwds.c.h @@ -33,15 +33,7 @@ lone_kwds(PyObject *module, PyObject *args, PyObject *kwargs) if (!_PyArg_NoPositional("lone_kwds", args)) { goto exit; } - if (kwargs == NULL) { - __clinic_kwds = PyDict_New(); - if (__clinic_kwds == NULL) { - goto exit; - } - } - else { - __clinic_kwds = Py_NewRef(kwargs); - } + __clinic_kwds = Py_XNewRef(kwargs); return_value = lone_kwds_impl(module, __clinic_kwds); exit: @@ -76,15 +68,7 @@ kwds_with_pos_only(PyObject *module, PyObject *args, PyObject *kwargs) } a = PyTuple_GET_ITEM(args, 0); b = PyTuple_GET_ITEM(args, 1); - if (kwargs == NULL) { - __clinic_kwds = PyDict_New(); - if (__clinic_kwds == NULL) { - goto exit; - } - } - else { - __clinic_kwds = Py_NewRef(kwargs); - } + __clinic_kwds = Py_XNewRef(kwargs); return_value = kwds_with_pos_only_impl(module, a, b, __clinic_kwds); exit: @@ -123,15 +107,7 @@ kwds_with_optional_pos_only(PyObject *module, PyObject *args, PyObject *kwargs) } b = PyTuple_GET_ITEM(args, 1); skip_optional: - if (kwargs == NULL) { - __clinic_kwds = PyDict_New(); - if (__clinic_kwds == NULL) { - goto exit; - } - } - else { - __clinic_kwds = Py_NewRef(kwargs); - } + __clinic_kwds = Py_XNewRef(kwargs); return_value = kwds_with_optional_pos_only_impl(module, a, b, __clinic_kwds); exit: @@ -160,18 +136,9 @@ kwds_with_stararg(PyObject *module, PyObject *args, PyObject *kwargs) PyObject *__clinic_kwds = NULL; __clinic_args = Py_NewRef(args); - if (kwargs == NULL) { - __clinic_kwds = PyDict_New(); - if (__clinic_kwds == NULL) { - goto exit; - } - } - else { - __clinic_kwds = Py_NewRef(kwargs); - } + __clinic_kwds = Py_XNewRef(kwargs); return_value = kwds_with_stararg_impl(module, __clinic_args, __clinic_kwds); -exit: /* Cleanup for args */ Py_XDECREF(__clinic_args); /* Cleanup for kwds */ @@ -211,15 +178,7 @@ kwds_with_pos_only_and_stararg(PyObject *module, PyObject *args, PyObject *kwarg if (!__clinic_args) { goto exit; } - if (kwargs == NULL) { - __clinic_kwds = PyDict_New(); - if (__clinic_kwds == NULL) { - goto exit; - } - } - else { - __clinic_kwds = Py_NewRef(kwargs); - } + __clinic_kwds = Py_XNewRef(kwargs); return_value = kwds_with_pos_only_and_stararg_impl(module, a, b, __clinic_args, __clinic_kwds); exit: @@ -230,4 +189,4 @@ kwds_with_pos_only_and_stararg(PyObject *module, PyObject *args, PyObject *kwarg return return_value; } -/*[clinic end generated code: output=27a7ba9440b9a884 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=ad46500147228faf input=a9049054013a1b77]*/ diff --git a/Modules/clinic/_zoneinfo.c.h b/Modules/clinic/_zoneinfo.c.h index 5d13e5b745562c2..b3c5defcbd937d1 100644 --- a/Modules/clinic/_zoneinfo.c.h +++ b/Modules/clinic/_zoneinfo.c.h @@ -425,15 +425,7 @@ zoneinfo_ZoneInfo___init_subclass__(PyObject *type, PyObject *args, PyObject *kw if (!_PyArg_NoPositional("__init_subclass__", args)) { goto exit; } - if (kwargs == NULL) { - __clinic_kwargs = PyDict_New(); - if (__clinic_kwargs == NULL) { - goto exit; - } - } - else { - __clinic_kwargs = Py_NewRef(kwargs); - } + __clinic_kwargs = Py_XNewRef(kwargs); return_value = zoneinfo_ZoneInfo___init_subclass___impl((PyTypeObject *)type, __clinic_kwargs); exit: @@ -442,4 +434,4 @@ zoneinfo_ZoneInfo___init_subclass__(PyObject *type, PyObject *args, PyObject *kw return return_value; } -/*[clinic end generated code: output=d7aeeec4b950fc35 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=42fad9178a9395ea input=a9049054013a1b77]*/ diff --git a/Modules/clinic/atexitmodule.c.h b/Modules/clinic/atexitmodule.c.h index 9afa3a15a29614e..8297f03c0276b59 100644 --- a/Modules/clinic/atexitmodule.c.h +++ b/Modules/clinic/atexitmodule.c.h @@ -39,15 +39,7 @@ atexit_register(PyObject *module, PyObject *args, PyObject *kwargs) if (!__clinic_args) { goto exit; } - if (kwargs == NULL) { - __clinic_kwargs = PyDict_New(); - if (__clinic_kwargs == NULL) { - goto exit; - } - } - else { - __clinic_kwargs = Py_NewRef(kwargs); - } + __clinic_kwargs = Py_XNewRef(kwargs); return_value = atexit_register_impl(module, func, __clinic_args, __clinic_kwargs); exit: @@ -58,4 +50,4 @@ atexit_register(PyObject *module, PyObject *args, PyObject *kwargs) return return_value; } -/*[clinic end generated code: output=8fd8f691fdbc4e0a input=a9049054013a1b77]*/ +/*[clinic end generated code: output=ab4b375d516aa59b input=a9049054013a1b77]*/ diff --git a/Objects/clinic/namespaceobject.c.h b/Objects/clinic/namespaceobject.c.h index cdd9ef586c79ecd..9f6301592d2c84c 100644 --- a/Objects/clinic/namespaceobject.c.h +++ b/Objects/clinic/namespaceobject.c.h @@ -25,15 +25,7 @@ namespace___replace__(PyObject *self, PyObject *args, PyObject *kwargs) if (!_PyArg_NoPositional("__replace__", args)) { goto exit; } - if (kwargs == NULL) { - changes = PyDict_New(); - if (changes == NULL) { - goto exit; - } - } - else { - changes = Py_NewRef(kwargs); - } + changes = Py_XNewRef(kwargs); return_value = namespace___replace___impl((_PyNamespaceObject *)self, changes); exit: @@ -42,4 +34,4 @@ namespace___replace__(PyObject *self, PyObject *args, PyObject *kwargs) return return_value; } -/*[clinic end generated code: output=67b5fcb6a6aee559 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=1b1c015f14360d64 input=a9049054013a1b77]*/ diff --git a/Objects/clinic/structseq.c.h b/Objects/clinic/structseq.c.h index f115f8ddcf490c6..ce5014311f4cca7 100644 --- a/Objects/clinic/structseq.c.h +++ b/Objects/clinic/structseq.c.h @@ -87,15 +87,7 @@ structseq___replace__(PyObject *self, PyObject *args, PyObject *kwargs) if (!_PyArg_NoPositional("__replace__", args)) { goto exit; } - if (kwargs == NULL) { - changes = PyDict_New(); - if (changes == NULL) { - goto exit; - } - } - else { - changes = Py_NewRef(kwargs); - } + changes = Py_XNewRef(kwargs); return_value = structseq___replace___impl((PyStructSequence *)self, changes); exit: @@ -104,4 +96,4 @@ structseq___replace__(PyObject *self, PyObject *args, PyObject *kwargs) return return_value; } -/*[clinic end generated code: output=18fcf88e5df26774 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=f0231faecca9248b input=a9049054013a1b77]*/ diff --git a/Objects/namespaceobject.c b/Objects/namespaceobject.c index e1ed2f6df9a6e21..6d5fced0ed73b35 100644 --- a/Objects/namespaceobject.c +++ b/Objects/namespaceobject.c @@ -261,7 +261,9 @@ namespace___replace___impl(_PyNamespaceObject *self, PyObject *changes) Py_DECREF(result); return NULL; } - if (PyDict_Update(((_PyNamespaceObject*)result)->ns_dict, changes) < 0) { + if (changes != NULL + && PyDict_Update(((_PyNamespaceObject*)result)->ns_dict, changes) < 0) + { Py_DECREF(result); return NULL; } diff --git a/Objects/structseq.c b/Objects/structseq.c index 36daf523d8004f1..949790e6f416e95 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -424,9 +424,10 @@ structseq___replace___impl(PyStructSequence *self, PyObject *changes) // We do not support types with unnamed fields, so we can iterate over // i >= n_visible_fields case without slicing with (i - n_unnamed_fields). for (i = 0; i < n_fields; ++i) { - PyObject *ob; - if (PyDict_PopString(changes, Py_TYPE(self)->tp_members[i].name, - &ob) < 0) { + PyObject *ob = NULL; + if (changes != NULL + && PyDict_PopString(changes, Py_TYPE(self)->tp_members[i].name, + &ob) < 0) { goto error; } if (ob == NULL) { @@ -435,7 +436,7 @@ structseq___replace___impl(PyStructSequence *self, PyObject *changes) result->ob_item[i] = ob; } // Check if there are any unexpected fields. - if (PyDict_GET_SIZE(changes) > 0) { + if (changes != NULL && PyDict_GET_SIZE(changes) > 0) { PyObject *names = PyDict_Keys(changes); if (names) { PyErr_Format(PyExc_TypeError, "Got unexpected field name(s): %R", names); diff --git a/Tools/clinic/libclinic/converters.py b/Tools/clinic/libclinic/converters.py index c54dab302b66586..e3571eb597f24af 100644 --- a/Tools/clinic/libclinic/converters.py +++ b/Tools/clinic/libclinic/converters.py @@ -1449,15 +1449,5 @@ def cleanup(self) -> str: return f'Py_XDECREF({self.parser_name});\n' def parse_var_keyword(self) -> str: - param_name = self.parser_name - return f""" - if (kwargs == NULL) {{{{ - {param_name} = PyDict_New(); - if ({param_name} == NULL) {{{{ - goto exit; - }}}} - }}}} - else {{{{ - {param_name} = Py_NewRef(kwargs); - }}}} - """ + # NULL when there are no keywords: no call allocates an empty dict. + return f"{self.parser_name} = Py_XNewRef(kwargs);\n"