-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce replacement API for PyIter_Next
with a non-ambiguous return value
#21
Comments
In this case, isn't it a complicating factor that the underlying |
I'll quote @markshannon in capi-workgroup/problems#1 (comment):
|
The current API ( |
I get that. But changing Well, it saves one call to |
Ah, yes that is true. I guess it boils down to a cosmetic issue, for now. I'll let the WG decide if a cleaner API is worth it. |
My personal view is that we have bigger fish to fry. |
+1 for the third one, it matches where we've been going so far and I don't see a reason to change course. But there are some details that make this fish not worth frying right now, like:
|
FWIW: yes, that would make for a saner public API.
What's the use-case? I could not find such a use-case in the code base. |
Consistency in whether NULL can be passed to an output parameter. (Deciding whether we want that -- and how to treat output parameters in general -- is one of the proverbial bigger fish to fry.) |
I feel it's better not to allow setting an output parameter's address to NULL. It's a rare case, and it slows down the API function (it has to check whether the pointer is NULL) and may bulk it up (with a DECREF, assuming the output value is produced anyways). Also, I feel that upon error the output parameter should always be set to NULL, to avoid potentially uninitialized memory. |
I like this API:
I agree 100% :-) We already did that in other recently added functions such as |
IMO it's worth it to propose a better API for PyIter_Next() which has an ambiguous return value (require to check for @erlend-aasland @gvanrossum @zooba: What do you think of the proposed API #21 (comment) ? I understand that @encukou likes it. Changing |
I like it. It makes this pattern work nicely (due to both errors and valid results coercing to PyObject *iter = PyObject_GetIter(o);
if (!iter) goto error;
PyObject *item = NULL;
while (PyIter_NextItem(iter, &item)) {
if (!item) goto error;
// work on item
Py_DECREF(item);
}
Py_DECREF(iter); |
I like it.
Yeah; it is unfortunate, but IMO it is not a blocker for providing a better |
Ok, let's do a formal vote on the API #21 (comment): |
See also: capi-workgroup/problems#29 |
Previous discussion:
I would like to pursue this issue. IMO,
PyIter_Next
having an ambiguous return value is a good enough motivation for adding a replacement API. If you disagree, feel free to just close this issue. AFAICS, three APIs have been discussed; I'm in favour of Mark's proposal (item 3):1:
PyObject *PyIter_NextItem(PyObject* iter, int *err)
Return values: a NULL pointer (loop termination or error) or a PyObject pointer (item returned)
Mentioned in python/cpython#105201 (comment)
Example usage
2:
int PyIter_NextItem(PyObject* iter, PyObject **item)
Return values: -1 (error) or 0 (item returned or loop termination)
Mentioned in python/cpython#105201 (comment)
Example usage
3:
int PyIter_NextItem(PyObject *iter, PyObject **next)
Return values: -1 (error), 0 (loop terminated), or 1 (item returned)
Mentioned in python/cpython#105201 (comment)
Example usage
The text was updated successfully, but these errors were encountered: