Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
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.
| @@ -6243,7 +6198,7 @@ slot_tp_hash(PyObject *self) | |||
| Py_ssize_t h; | |||
| 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().
| @@ -6422,7 +6377,7 @@ slot_tp_richcompare(PyObject *self, PyObject *other, int op) | |||
| int unbound; | |||
| 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().
| @@ -6441,7 +6396,7 @@ slot_tp_iter(PyObject *self) | |||
| PyObject *func, *res; | |||
| _Py_IDENTIFIER(__iter__); | |||
|
|
|||
| func = lookup_method(self, &PyId___iter__, &unbound); | |||
| func = lookup_maybe_method(self, &PyId___iter__, &unbound); | |||
| @@ -6616,7 +6571,7 @@ slot_am_aiter(PyObject *self) | |||
| PyObject *func, *res; | |||
| _Py_IDENTIFIER(__aiter__); | |||
|
|
|||
| func = lookup_method(self, &PyId___aiter__, &unbound); | |||
| func = lookup_maybe_method(self, &PyId___aiter__, &unbound); | |||
| @@ -6635,7 +6590,7 @@ slot_am_anext(PyObject *self) | |||
| PyObject *func, *res; | |||
| _Py_IDENTIFIER(__anext__); | |||
|
|
|||
| func = lookup_method(self, &PyId___anext__, &unbound); | |||
| func = lookup_maybe_method(self, &PyId___anext__, &unbound); | |||
Objects/typeobject.c
Outdated
| 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"
| @@ -6223,7 +6178,7 @@ slot_tp_repr(PyObject *self) | |||
| _Py_IDENTIFIER(__repr__); | |||
| 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: |
No description provided.