Fix Code.getargs() treating co_flags bitmask values as counts#14493
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
|
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.
There was a problem hiding this comment.
Good point, updated the changelog to mention the kwonly fix.
RonnyPfannschmidt
left a comment
RonnyPfannschmidt
left a comment
There was a problem hiding this comment.
looks good now
do you want to squash or should we
@nicoddemus im on the fence on whether this should be marked for backlog
|
feel free to squash on merge — whatever's easier on your side. |
|
Thanks @EternalRights!
Seems OK to backport, I will do it. 👍 |
nicoddemus
added
the
backport 9.0.x
Backport to 9.0.x: 💚 backport PR created✅ Backport PR branch: Backported as #14535 🤖 @patchback |
#14535) 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 (cherry picked from commit f976bb6) Co-authored-by: EternalRights <162705204+EternalRights@users.noreply.github.com>
As a follow-up to pytest-dev#14493, replace direct co_* accesses in Code.path and Code.getargs with inspect.getfile and inspect.getfullargspec respectively, delegating argument-decoding logic to well-tested stdlib APIs rather than re-implementing it with raw bitmask arithmetic. Code.firstlineno and Code.name still access co_firstlineno and co_name directly: the inspect module has no equivalent for these that does not require reading the source file from disk, which would regress correctness for compile()'d code and performance for every traceback formatting path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
4 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