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
Inconsistent exception usage in PyLong_As* C functions #57118
Comments
The C functions for converting a Python 'int' object to a C integer are Raising a SystemError here is quite unhelpful. My understanding is that Is there any reason not to change the behaviour of these two functions to |
+1 for turning these into TypeErrors. It makes little sense that PyLong_AsLongLong and PyLong_AsUnsignedLongLong behave differently here. Do you have a patch handy? |
See attached. |
This probably shouldn't be backported to 3.2; it could break 3rd-party Also, it's worth noting that the error handling between conversion |
Agreed; I don't see this as a bugfix (especially since AFAIK it's not documented that TypeError should be raised here); rather, as a design improvement.
True; there are a number of inconsistencies left. We'll get them all eventually. :-) |
The patch still needs tests (e.g., in test_capi). I'm not sure whether it would be good to add information about the TypeError to the docs. |
Attached is an updated patch with tests. There don't seem to be any tests for PyLong_AsS[s]ize_t() and
Yeah, that doesn't seem necessary; raising TypeError in this sort of |
Looks good to me. I'd prefer 'test_long_as_size' to be called 'test_long_as_size_t' (even though that's inaccurate for the ssize_t bits :-). The 'Py_None' reference counting in test_long_as_size and test_long_as_double looked a little odd at first glance---particularly the lack of a Py_INCREF just before the return Py_None. I see that it works; it just caught my eye. Anyway, those are just nitpicks; I leave it to you whether you want to change anything. Otherwise, please go ahead and apply. Thanks for the patches! |
New changeset 4a66a35da3fd by Nadeem Vawda in branch 'default': |
Indeed, it is rather roundabout, so I added a comment to avoid confusion.
Thanks for the review. |
+1 |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: