Sorry, something went wrong.
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead. |
Sorry, something went wrong.
Sorry, something went wrong.
|
The current code covers all combinations of plain_names & names_with_default being NULL and non-NULL. Removing one of the cases breaks that, leaving no hint about why this would fall through to the empty posargs case. // With the current grammar, we never get here.
// When that changes, remove the assert, and test thoroughly.
assert(0);
*posargs = plain_names;
|
Sorry, something went wrong.
|
I'd replace the branch with something like: I agree, that's better. Should I resubmit? |
Sorry, something went wrong.
Sorry, something went wrong.
|
This looks good to me, thanks! |
Sorry, something went wrong.
|
Thanks! I meant to wrap this up over the weekend, but forgot. I will do it this week. |
Sorry, something went wrong.
|
@encukou I split the changes into two commits, one to improve coverage and the other to reorganize the if-else structure. |
Sorry, something went wrong.
|
Thanks! For the future: please avoid force-pushing to CPython -- when reviewed commits disappear, the review needs to be restarted. (In this commit it's not a problem, as it's just a few lines.) |
Sorry, something went wrong.
|
Thanks again for your help and guidance through the whole process. |
Sorry, something went wrong.
The coverage report indicates that this branch is never exercised. That appears to be because it is unused code. When I couldn't produce a test case that exercised it, I removed the branch, then checked that the complete test suite still passed.
The branch is only run when names_with_default is null. It appears that when there are no names with defaults, names_with_defaults will point to a structure that has .size zero.
It's possible that a future implementation change might alter that, requiring the branch to be put back, but I believe the existing tests would detect if that happened.