gh-94808: Remove unused code in _make_posargs
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.
- Issue: gh-94808
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.
@encukou
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.
If it was me, I'd replace the branch with something like:
// With the current grammar, we never get here.
// When that changes, remove the assert, and test thoroughly.
assert(0);
*posargs = plain_names;
I'd replace the branch with something like:
I agree, that's better. Should I resubmit?
This looks good to me, thanks! I see you've marked it as draft, do you still want to make changes?
Thanks! I meant to wrap this up over the weekend, but forgot. I will do it this week.
@encukou I split the changes into two commits, one to improve coverage and the other to reorganize the if-else structure.
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.) Reorganizing the commits doesn't help, since all PRs are squashed when merging.
Thanks again for your help and guidance through the whole process.