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
gh-91625: Don't ignore extended args of adaptive opcodes #91626
Conversation
Misc/NEWS.d/next/Core and Builtins/2022-04-17-02-55-38.gh-issue-91625.80CrC7.rst
Outdated
Show resolved
Hide resolved
Python/ceval.c
Outdated
assert(cframe.use_tracing == 0 || cframe.use_tracing == 255); \ | ||
opcode |= cframe.use_tracing OR_DTRACE_LINE; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the tracing checks here should be removed. We should be able to finish executing the logical opcode (family) we started to execute before trying to respect cframe.use_tracing.
Do reviewers agree?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only time it could make a difference is if one of the _Py_Specialize_XxxXxxxx()
calls somehow managed to set the flag.
Thanks for catching this.
The tests could be tidied up with a bit of code generation. Other than that, looks good.
Lib/test/test_descr.py
Outdated
def __getattr__(self, attr): | ||
return int(attr.lstrip("_")) | ||
def number_attrs(Z): | ||
return [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks ideal for generating. Maybe something like
attrs = ", ".join(f"Z._{n:03d}" for n in range(280))
code = f"def number_attrs(Z):\n return [ {attrs} ]"
exec(code...
Lib/test/test_unpack.py
Outdated
@@ -150,6 +150,45 @@ def load_tests(loader, tests, pattern): | |||
tests.addTest(doctest.DocTestSuite()) | |||
return tests | |||
|
|||
def unpack_400(x): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generate this as well?
#91625