Skip to content

gh-146480: Override the exception in _PyErr_SetKeyError() #146486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vstinner merged 4 commits into from
Mar 27, 2026
Merged

gh-146480: Override the exception in _PyErr_SetKeyError() #146486

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
43b72b0
gh-146480: Override the exception in _PyErr_SetKeyError()
vstinner Mar 26, 2026
36d6f12
Add more tests
vstinner Mar 26, 2026
30212a2
Address Serhiy's new review
vstinner Mar 27, 2026
03e509e
Add missing import _testinternalcapi
vstinner Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Lib/test/test_capi/test_misc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -928,6 +928,15 @@ def test_tp_bases_slot_none(self):
_testcapi.create_heapctype_with_none_bases_slot
)

def test__pyerr_setkeyerror(self):
# Test _PyErr_SetKeyError()
_pyerr_setkeyerror = _testinternalcapi._pyerr_setkeyerror
with self.assertRaises(KeyError) as cm:
# Test _PyErr_SetKeyError() with an exception set to check
# that the function overrides the current exception
_pyerr_setkeyerror("key")
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is worth to test also the case when the argument is a tuple and when the argument is a KeyError instance. I think this was the reason of introducing _PyErr_SetKeyError() instead of simply calling PyErr_SetObject().

Copy link
Copy Markdown
Member Author

@vstinner vstinner Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I added tests on a tuple and a KeyError.

I think this was the reason of introducing _PyErr_SetKeyError() instead of simply calling PyErr_SetObject().

Ah right, PyErr_SetObject(exc_type, value) has an unsual API: if value is a tuple, it unpacks arguments by calling exc_type(*value). Otherwise, it calls exc_type(value).

Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if the value is already an instance of the specified exception type, it just uses it.

self.assertEqual(cm.exception.args, ("key",))


@requires_limited_api
class TestHeapTypeRelative(unittest.TestCase):
Expand Down
15 changes: 15 additions & 0 deletions Modules/_testinternalcapi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2838,6 +2838,20 @@ test_threadstate_set_stack_protection(PyObject *self, PyObject *Py_UNUSED(args))
}


static PyObject *
_pyerr_setkeyerror(PyObject *self, PyObject *arg)
{
// Test that _PyErr_SetKeyError() overrides the current exception
// if an exception is set
PyErr_NoMemory();

_PyErr_SetKeyError(arg);

assert(PyErr_Occurred());
return NULL;
}


static PyMethodDef module_functions[] = {
{"get_configs", get_configs, METH_NOARGS},
{"get_eval_frame_stats", get_eval_frame_stats, METH_NOARGS, NULL},
Expand Down Expand Up @@ -2959,6 +2973,7 @@ static PyMethodDef module_functions[] = {
{"module_get_gc_hooks", module_get_gc_hooks, METH_O},
{"test_threadstate_set_stack_protection",
test_threadstate_set_stack_protection, METH_NOARGS},
{"_pyerr_setkeyerror", _pyerr_setkeyerror, METH_O},
{NULL, NULL} /* sentinel */
};

Expand Down
10 changes: 9 additions & 1 deletion Python/errors.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,19 @@ PyErr_SetObject(PyObject *exception, PyObject *value)

/* Set a key error with the specified argument, wrapping it in a
* tuple automatically so that tuple keys are not unpacked as the
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is outdated, because the code no longer explicitly wraps the key in a tuple. But it has roughtly the same effect, the reasons why it does not simply use PyErr_SetObject() is still valid. If we keep the current code, we need other wording here.

I wonder if partial reversion of aa36f83 will fix the issue. Would it have performance impact? This is just a curiosity, not suggestion.

Copy link
Copy Markdown
Member Author

@vstinner vstinner Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the comment by removing the part about wrapping into a tuple.

I wonder if partial reversion of aa36f83 will fix the issue.

The old implementation can be called with an exception set:

void
_PyErr_SetKeyError(PyObject *arg)
{
    PyThreadState *tstate = _PyThreadState_GET();
    PyObject *tup = PyTuple_Pack(1, arg);
    if (!tup) {
        /* caller will expect error to be set anyway */
        return;
    }
    _PyErr_SetObject(tstate, PyExc_KeyError, tup);
    Py_DECREF(tup);
}

Would it have performance impact? This is just a curiosity, not suggestion.

According to my comment, I rewrote _PyErr_SetKeyError() to be able to reuse the faster BaseException_vectorcall(). I don't know if it has a significant impact on performance. This vectorcall function has to create a tuple of arguments using _PyTuple_FromArray(args, PyVectorcall_NARGS(nargsf)).

* exception arguments. */
* exception arguments.
*
* If an exception is already set, override the exception. */
void
_PyErr_SetKeyError(PyObject *arg)
{
PyThreadState *tstate = _PyThreadState_GET();

// PyObject_CallOneArg() must not be called with an exception set,
// otherwise _Py_CheckFunctionResult() can fail if the function returned
// a result with an excception set.
_PyErr_Clear(tstate);

PyObject *exc = PyObject_CallOneArg(PyExc_KeyError, arg);
if (!exc) {
/* caller will expect error to be set anyway */
Expand Down
Loading