Fix Code.getargs() treating co_flags bitmask values as counts#14493
Fix Code.getargs() treating co_flags bitmask values as counts#14493EternalRights wants to merge 5 commits into
Conversation
CO_VARARGS (4) and CO_VARKEYWORDS (8) are bitmask flags, but they were used directly as increment values for the argument count via `&`. This caused co_varnames[:argcount] to slice beyond the actual arguments when the function body contained enough local variables. Fix by wrapping the bitmask results in bool(), so they contribute at most 1 to the argcount. Closes pytest-dev#14492
psf-chronographer
Bot
added
the
bot:chronographer:provided
for more information, see https://pre-commit.ci
| if var: | ||
| argcount += raw.co_flags & CO_VARARGS | ||
| argcount += raw.co_flags & CO_VARKEYWORDS | ||
| argcount += bool(raw.co_flags & CO_VARARGS) |
There was a problem hiding this comment.
I fear this may get confused by mixing in kwonly args
|
You were right about kwonly args. The layout of Pushed a fix that adds ptal. |
|
man, this one is a messy piece with a history - i wonder if signature could be used in more modern code to just use the names from the signature that being said - the remenants of |
yeah this needs more thought. I’ll come back to it tomorrow. |
co_varnames layout is: positional args, kwonly args, *args, **kwargs, locals. The f6 test expected the wrong order. Also suppress ruff F841 false positives on locals deliberately placed to test co_varnames slicing.
Codecov reports coverage for testing/ files since the project config includes them. The function bodies of f5/f6 (local var assignments) were never executed because the test only used Code.from_function(), never called the functions. Replace raise NotImplementedError() with actual calls to cover all diff lines.
|
kwonly args are now accounted for and CI is all green. The |
| @@ -0,0 +1 @@ | |||
| Fixed ``Code.getargs()`` incorrectly including local variable names in the returned argument tuple for functions with ``*args`` and/or ``**kwargs``. The method was using ``co_flags`` bitmask values (``4`` and ``8``) directly as counts instead of converting them to ``1`` via ``bool()``. | |||
There was a problem hiding this comment.
Could the changelog entry also mention the kwonly fix?
account for co_kwonlyargcount in getargs when var=True
Right now it only talks about the bitmask, but someone affected by the kwonly case wouldn't find their bug in the release notes.
3 participants
CO_VARARGS (4) and CO_VARKEYWORDS (8) are bitmask flags, but they were used directly as increment values for the argument count via
&. This caused co_varnames[:argcount] to slice beyond the actual arguments when the function body contained enough local variables.Fix by wrapping the bitmask results in bool(), so they contribute at most 1 to the argcount.
Closes #14492