Repository navigation
Add PyMapping_GetOptionalItem() #106307
Description
PyObject_GetItem() raises a KeyError if the key is not found in a mapping. In some cases it should be treated as any other error, but in other cases it should be caught and suppressed. The repeating pattern of PyObject_GetItem() followed by PyErr_ExceptionMatches(PyExc_KeyError) and PyErr_Clear() occurs 7 times in Python/bytecodes.c and at least 5 times in other files.
I propose to add private helper _PyMapping_LookupItem() which combines these three calls to make the code clearer. It also has a special case for exact dict, so eliminates even more repeating code. For example:
PyObject *m;
- if (PyDict_CheckExact(modules)) {
- m = Py_XNewRef(PyDict_GetItemWithError(modules, name));
- }
- else {
- m = PyObject_GetItem(modules, name);
- if (_PyErr_ExceptionMatches(tstate, PyExc_KeyError)) {
- _PyErr_Clear(tstate);
- }
- }
- if (_PyErr_Occurred(tstate)) {
+ if (_PyMapping_LookupItem(modules, name, &m) < 0) {
return NULL;
}The interface of _PyMapping_LookupItem() is very similar to other private helper _PyObject_LookupAttr() (see #76752) and to a proposed new C API for PyDict (see #106004).
Activity
pythongh-106307: Add _PyMapping_LookupItem()
pythongh-106307: Add _PyMapping_LookupItem()
carljm commentedon Jul 3, 2023
Why should this be a private API? It seems like for many uses it's simply better, and it doesn't seem to expose any internal implementation details that we would be hesitant to support.
I think
_PyObject_LookupAttrshould probably be made public, in which case the leading underscore will be a wart that would be painful to remove but potentially confusing. Why not introducePyMapping_LookupItemwithout the leading underscore in the first place?serhiy-storchaka commentedon Jul 6, 2023
This is an interesting idea. If make
_PyObject_LookupAttrpublic, then_PyMapping_LookupItemcan be made public from the start.[-]Add _PyMapping_LookupItem()[/-][+]Add PyMapping_GetOptionalItem()[/+]gh-106307: C API: Add PyMapping_GetOptionalItem() function (GH-106308)
vstinner commentedon Jul 12, 2023
I added the 2 functions to pythoncapi-compat: python/pythoncapi-compat@7515dae
pythongh-106307: Fix PyMapping_GetOptionalItemString()
gh-106307: Fix PyMapping_GetOptionalItemString() (GH-108797)