-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
PEP 590: Correct sign of return value of PyVectorcall_NARGS(). #1105
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
Conversation
|
I argue that it should remain |
|
For future reference: I think it's good to put all authors of a PEP in CC when proposing a change to a PEP. |
|
We all agreed that the size of |
As you can see from #1066 (comment), this was intentional. We didn't forget to change the return type. |
I don't see why. It's a cast from an unsigned number to a signed number of the same size. This doesn't change the bit pattern of the number, so why there should there be any overhead? To support |
|
Compare the GCC output for the form with and without the cast: https://www.godbolt.org/z/GG3Pmy
|
Comparing unoptimized code is not very relevant. When you compile even with |
Maybe you're right. I'm not even arguing about that. I'm just saying that using |
By the way: that looks like a very cool site, I didn't know about it. Thanks! |
|
Indeed, FWIW, I grew to like the practical hack: as it's done now, you're likely to get a compiler warning when mixing "arguments with the offset flag" and "just number of arguments" inappropriately. |
+1 |
|
Can we get some resolution on this please? There is no point in keeping this PR open if it doesn't look like it will be accepted. |
|
Yes. Please re-open if there's more to discuss. (Or comment, if you can't reopen.) |
The return type of
PyVectorcall_NARGS()is documented atPy_ssize_tbut it makes no sense for this to be a signed value, for two reasons.