Skip to content

gh-91625: Don't ignore extended args of adaptive opcodes - #91626

Merged
sweeneyde merged 12 commits into
python:mainfrom
sweeneyde:extend
Apr 17, 2022
Merged

gh-91625: Don't ignore extended args of adaptive opcodes#91626
sweeneyde merged 12 commits into
python:mainfrom
sweeneyde:extend

Conversation

@sweeneyde

@sweeneyde sweeneyde commented Apr 17, 2022

Copy link
Copy Markdown
Member

Comment thread Misc/NEWS.d/next/Core and Builtins/2022-04-17-02-55-38.gh-issue-91625.80CrC7.rst Outdated
@sweeneyde sweeneyde added type-bug An unexpected behavior, bug, or error interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.11 only security fixes labels Apr 17, 2022
Comment thread Python/ceval.c Outdated
Comment on lines +1358 to +1359
assert(cframe.use_tracing == 0 || cframe.use_tracing == 255); \
opcode |= cframe.use_tracing OR_DTRACE_LINE; \

@sweeneyde sweeneyde Apr 17, 2022

Copy link
Copy Markdown
Member Author

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?

Copy link
Copy Markdown
Member Author

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.

@markshannon markshannon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this.

The tests could be tidied up with a bit of code generation. Other than that, looks good.

Comment thread Lib/test/test_descr.py Outdated
def __getattr__(self, attr):
return int(attr.lstrip("_"))
def number_attrs(Z):
return [

Copy link
Copy Markdown
Member

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...

Comment thread Lib/test/test_unpack.py Outdated
tests.addTest(doctest.DocTestSuite())
return tests

def unpack_400(x):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generate this as well?

@sweeneyde
sweeneyde merged commit cec5d85 into python:main Apr 17, 2022
@sweeneyde
sweeneyde deleted the extend branch April 24, 2022 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) release-blocker type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants