Implement PyObject_HasOptionalAttr

[MightyJosip]

Feature or enhancement

Proposal:

Looking at the hasattr(obj) source code I noticed it calls PyObject_GetOptionalAttr

cpython/Python/bltinmodule.c

Line 1244 in 9c1dfe2

if (PyObject_GetOptionalAttr(obj, name, &v) < 0) {

But that value (reference to attribute) is never used. My idea is to create the new function that doesn't set the reference and everything, but rather just returns 0 (if error happened or attribute doesn't exist), or 1 (if attribute exists).

2 more things.

First inside of PyObject_GetOptionalAttr function PyUnicode_Check is called more than once (both at the start of the function, and in other functions later (like in _PyObject_GenericGetAttrWithDict)

cpython/Objects/object.c

Line 1188 in 9c1dfe2

PyObject_GetOptionalAttr(PyObject *v, PyObject *name, PyObject **result)

So does PyUnicode_Check has performance implications, can hasattr/getattr be faster if it is called only once.

And second. Does the documentation for hasattr need improvement

cpython/Python/bltinmodule.c

Lines 1233 to 1235 in 9c1dfe2

	Return whether the object has an attribute with the given name.
	This is done by calling getattr(obj, name) and catching AttributeError.

Since with introduction of PyObject_GetOptionalAttr it doesn't raise AttributeError.

This issue report is made purely as possible performance improvement question, and should be treated like that, if it is something that shouldn't be done feel free to close the issue, otherwise I would gladly try to implement the proposed change

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

No response

[rruuaanng]

I think it's a good idea to make the implementation more intuitive, and the user can easily associate it with Hhas when using it, rather than checking by not getting the error value returned.
But it seems that the goal of CAPI is to be consistent with python. That doesn't seem like much of a point.

[rruuaanng]

cc @vstinner

[serhiy-storchaka]

My idea is to create the new function that doesn't set the reference and everything, but rather just returns 0 (if error happened or attribute doesn't exist), or 1 (if attribute exists).

This is implemented as PyObject_HasAttrWithError.

So does PyUnicode_Check has performance implications, can hasattr/getattr be faster if it is called only once.

Good observation. The PyUnicode_Check() check is needed before calling tp_getattro() or tp_getattr(). It can be moved after the branch for _PyObject_GenericGetAttrWithDict(), but I am not sure that this is worth a hassle. It is cheap, especially in the context of dict lookup, and there should be at least yet one check in the dict lookup. _PyObject_GenericGetAttrWithDict() is called in several other places after the PyUnicode_Check() check, so we could remove the check from _PyObject_GenericGetAttrWithDict(). On other hand, removing the check where it was needed can introduce a bug. So it is safer to keep all as is. Nex refactoring can change this.

Does the documentation for hasattr need improvement

Its behavior was not changed. It never raised AttributeError. Before introduction of public PyObject_GetOptionalAttr() it used other, private, function. And before this it just called PyObject_GetAttr() and silenced an AttributeError. It is documented not in terms of the C API, and semantically it is equivalent to use of getattr() with catching an AttributeError.

[vstinner]

This is implemented as PyObject_HasAttrWithError.

Since the function is already implemented, can we close the issue?

@serhiy-storchaka replied to other suggestions.