bpo-30509: Clean up calling type slots. #1883
Conversation
|
@serhiy-storchaka, thanks for your PR! By analyzing the history of the files in this pull request, we identified @tim-one, @benjaminp and @larryhastings to be potential reviewers. |
Objects/typeobject.c
Outdated
|
|
||
| retval = PyObject_CallFunctionObjArgs(func, ival, NULL); | ||
| Py_DECREF(func); | ||
| retval = call_method(self, &PyId___getitem__, &ival, 1); |
There was a problem hiding this comment.
This change optimizes the __getitem__ slot if I understand correctly, since it handles bound/unbound methods. Good :-) It might be useful to mention it.
About &ival: FYI I removed the _PyObject_CallArg1() macro since it increased the stack usage: https://bugs.python.org/issue28858
Objects/typeobject.c
Outdated
| PyErr_SetObject(PyExc_AttributeError, getitem_str); | ||
| PyObject *retval; | ||
| PyObject *ival = PyLong_FromSsize_t(i); | ||
| if (ival == NULL) |
There was a problem hiding this comment.
PEP 7 now requires { ... } for if block.
| int unbound; | ||
|
|
||
| func = lookup_method(self, &PyId___hash__, &unbound); | ||
| func = lookup_maybe_method(self, &PyId___hash__, &unbound); |
There was a problem hiding this comment.
ok: Error handled below by PyObject_HashNotImplemented().
| PyObject *func, *res; | ||
|
|
||
| func = lookup_method(self, &name_op[op], &unbound); | ||
| func = lookup_maybe_method(self, &name_op[op], &unbound); |
There was a problem hiding this comment.
ok: exception ignored, PyErr_Clear().
| _Py_IDENTIFIER(__iter__); | ||
|
|
||
| func = lookup_method(self, &PyId___iter__, &unbound); | ||
| func = lookup_maybe_method(self, &PyId___iter__, &unbound); |
| _Py_IDENTIFIER(__aiter__); | ||
|
|
||
| func = lookup_method(self, &PyId___aiter__, &unbound); | ||
| func = lookup_maybe_method(self, &PyId___aiter__, &unbound); |
| _Py_IDENTIFIER(__anext__); | ||
|
|
||
| func = lookup_method(self, &PyId___anext__, &unbound); | ||
| func = lookup_maybe_method(self, &PyId___anext__, &unbound); |
| if (func == NULL) { | ||
| if (!PyErr_Occurred()) | ||
| PyErr_SetObject(PyExc_AttributeError, name->object); | ||
| func = lookup_method(obj, name, &unbound); |
There was a problem hiding this comment.
ok: reuse lookup_method() which handles exceptions the same way.
Objects/typeobject.c
Outdated
| if (!PyErr_Occurred()) | ||
| PyErr_SetObject(PyExc_AttributeError, name->object); | ||
| func = lookup_method(obj, name, &unbound); | ||
| if (func == NULL) |
There was a problem hiding this comment.
PEP 7: need { ... }
"braces are strongly preferred" https://www.python.org/dev/peps/pep-0007/
There was a problem hiding this comment.
"... but may be omitted where C permits"
| int unbound; | ||
|
|
||
| func = lookup_method(self, &PyId___repr__, &unbound); | ||
| func = lookup_maybe_method(self, &PyId___repr__, &unbound); |
There was a problem hiding this comment.
ok: exception ignored using PyErr_Clear().
|
LGTM except of missing { ... } at two places. |
|
AppVeyor failure is unrelated to this change: |
4 participants
No description provided.