Replacements for PyMapping_HasKey(), PyMapping_HasKeyString(), PyObject_HasAttr(), and PyObject_HasAttrString()

Core Development C API
1

Functions PyMapping_HasKey(), PyMapping_HasKeyString(), PyObject_HasAttr(), and PyObject_HasAttrString() have a flaw – they clear all errors raised inside (in custom __getitem__, __eq__, __hash__ or __getattr__, unhashable key or just a memory error). The similar flaw in hasattr() was fixed in Python 3, but these C API functions have no way to report an error.

So we need new functions in replacement. What should be their names? PyDict_GetItemWithError() replaced PyDict_GetItem() which has the same flaw. Should we add the WithError suffix in new names? Or use other suffix, Ex or 2 as in some other C API?

2

I’d prefer the explicit WithError suffix to the more cryptic Ex and 2 suffixes.

3 Likes
3

Would it be possible to change the function to start reporting errors? Is it going to break C extensions?

If it’s not possible, WithError is sadly the least bad suffix IMO. I concur with Erlend here.

1 Like
4

I think it is impossible. The code which uses PyMapping_HasKey() most likely will interpret the returned -1 as a true value. Currently it gets a false value, so this is a surprising change in behavior. It can follow different branch of code after this. It also leaves a raised exception, so the following successful return from the function can cause a crash or SystemError.

I get rid of most of uses of these functions in CPython by replacing them with private functions which finally found way in the limited C API under names PyObject_GetOptionalAttr() and PyMapping_GetOptionalItem(). The reason why I have not added the replacements earlier is that I was not sure that we need special functions for testing the existence of the attribute or the key, if the above functions can be used for this. But they need a variable to store the retrieved value and the following Py_DECREF(), so special *Has* functions are more convenient in some cases. It is also easier to port code from using PyObject_HasAttr() to use PyObject_HasAttrWithError() than to use PyObject_GetOptionalAttr().

if (PyObject_HasAttr(obj, attrname)) {
    // found
}
else {
    // not found
}

to

int rc = PyObject_HasAttrWithError(obj, attrname);
if (rc < 0) {
    // error
}
else if (rc) {
    // found
}
else {
    // not found
}

or

if (PyObject_HasAttrWithError(obj, attrname) > 0) {
    // found
}
else if (PyErrOccurred()) {
    // error
}
else {
    // not found
}

instead of

PyObject *tmp;
if (PyObject_GetOptionalAttr(obj, attrname, &tmp) < 0) {
    // error
}
else if (tmp) {
    Py_DECREF(tmp);
    // found
}
else {
    // not found
}

or

PyObject *tmp;
int rc = PyObject_GetOptionalAttr(obj, attrname, &tmp);
if (rc < 0) {
    // error
}
else if (rc) {
    Py_DECREF(tmp);
    // found
}
else {
    // not found
}

We can of course make PyObject_GetOptionalAttr() accepting NULL, so that PyObject_GetOptionalAttr(obj, attrname, NULL) is the same as PyObject_HasAttrWithError(obj, attrname), but it adds an overhead to every call or PyObject_GetOptionalAttr() and just look more ugly. We have PyDict_Contains() despite the existence of a number of PyDict_Get*() functions.

1 Like
5

Oh, there is the “C API” sub-category. Moved this topic there.

Here is an implementation.

It includes changes in the code to use PyObject_HasAttrWithError() where it is appropriate. There are no use cases for PyMapping_HasKeyWithError() in the CPython code.