Functions must not suppress exceptions

The following functions suppress arbitrary exceptions raised inside. In some cases they can also clear exceptions raised before the call.

PyDict_GetItem and PyDict_GetItemString
PyMapping_HasKey and PyMapping_HasKeyString
PyObject_HasAttr and PyObject_HasAttrString
PySys_GetObject

They can call Python code (via __hash__, __eq__, __getattr__ or __getattribute__) which can raise arbitrary exception which will be silenced anyway. PyDict_GetItem has a counterpart which keeps exception (PyDict_GetItemWithError), but other functions do not have any.

These function should not be used in modern code, except in cases in which any error is silenced anyway (and it is bad that we still have such places). They should be deprecated in future, both at compile time and at run time. But first we should introduce replacements. Should we just introduce new functions with WithError suffix? In CPython code all occurrences of PyObject_HasAttr were replaced with _PyObject_LookupAttr.

Historical reference: hasattr() never raised exceptions in Python 2.

Yes, PyIter_Next is unrelated.

It was unclear to me what was the discussed problem, so I changed the title to: "Functions must not suppress exceptions". Is it what you mean?

Is the problem that if a function is called with an exception set, the function can supress it?

The problem is that if an unexpected exception is raised inside a function, it should be reported to the caller. Functions mentioned above have no way to do this, and leaving exception set after the function would break an existing code down the line.

We should introduce the replacement C API. For example, PyObject_GetAttr() can be used instead of PyObject_HasAttr().

if (PyObject_HasAttr(obj, attr_name)) {
    // has attribute
}
else {
    // does not have attribute
}

should be correctly rewritten as

PyObject *dummy = PyObject_GetAttr(obj, attr_name);
if (dummy != NULL) {
    Py_DECREF(dummy);
    // has attribute
}
else if (PyErr_ExceptionMatches(PyExc_AttributeError)) {
    PyErr_Clear();
    // does not have attribute
}
else {
    // error
}

The new function PyObject_GetOptionalAttr() is more convenient, allowing to get rid of PyErr_ExceptionMatches(PyExc_AttributeError) and PyErr_Clear():

PyObject *dummy;
int rc = PyObject_GetOptionalAttr(obj, attr_name, &dummy);
if (rc < 0) {
    // error
}
else if (rc) {
    Py_DECREF(dummy);
    // has attribute
}
else {
    // does not have attribute
}

Perhaps it is worth to introduce yet one new function PyObject_HasAttrEx() to get rid of the dummy variable:

int rc = PyObject_HasAttrEx(obj, attr_name);
if (rc < 0) {
    // error
}
else if (rc) {
    // has attribute
}
else {
    // does not have attribute
}

Another option is to allow the result pointer to be NULL, in which case it won't be set/incref'd.
(Unlike an “input” argument, users aren't likely to pass NULL by mistake here.)

int rc = PyObject_GetOptionalAttr(obj, attr_name, NULL);
if (rc < 0) {
    // error
}
else if (rc) {
    // has attribute
}
else {
    // does not have attribute
}

The old Lookup name suggestion would fit this case better, though.

It would add a tiny overhead for every use of PyObject_GetOptionalAttr():

    if (result != NULL) {
        result = value;
    }

instead of simply

    result = value;

@markshannon

Will that even be measurable?

Edit: We should find the best API for users, then make it fast.
If performance matters here, IMO we should make the function (or relevant parts of it) inline and let the compiler sort it out. (Obviously, this'll help C/C++ and version-specific ABI only, but IMO that's fine).
cc @markshannon

I would prefer to store the information "has an attibute" in a separated variable:

int has_attr;
if (PyObject_HasAttrEx(obj, attr_name, &has_attr) < 0) {
    // error
    return -1;
}

if (has_attr) {
    // has attribute
}
else {
    // does not have attribute
}

About the Ex suffix, I created #52 about naming convention to add new C API functions. So far, I didn't see a clear consensus.

int rc = PyObject_GetOptionalAttr(obj, attr_name, NULL);

Hum, I dislike passing NULL to a function to get a different behavior. It reminds me #47 problem (even if here it's a little bit different).

I would prefer adding new functions for "hasattr".

If you change PySys_GetObject(), please return a new strong reference in the new API.

Hum, I dislike passing NULL to a function to get a different behavior. It reminds me #47 problem (even if here it's a little bit different).

IMO, the main point of #47 -- "what if creating a value fails and the code pass NULL instead of a Python object" -- doesn't apply at all here. Is there any other danger?

Proposed guideline issue: capi-workgroup/api-evolution#35

@serhiy-storchaka

Proposed guideline issue: capi-workgroup/api-evolution#35

Let's continue the discussion there, I close the issue.

@serhiy-storchaka added multiple variants of listed functions which don't suppress exceptions anymore.

By the way, see also capi-workgroup/decisions#21 about PyIter_Next().

Navigation Menu

Repository navigation

Functions must not suppress exceptions #51

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Functions must not suppress exceptions #51

Description

Activity

davidhewitt commented on Jun 25, 2023

serhiy-storchaka commented on Jul 2, 2023

vstinner commented on Jul 11, 2023

serhiy-storchaka commented on Jul 12, 2023

encukou commented on Jul 12, 2023

serhiy-storchaka commented on Jul 12, 2023

encukou commented on Jul 12, 2023

vstinner commented on Jul 12, 2023

vstinner commented on Jul 12, 2023

vstinner commented on Jul 12, 2023

encukou commented on Jul 12, 2023

encukou commented on Oct 24, 2023

vstinner commented on Jun 10, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions