Skip to content

Conversation

@markshannon
Copy link
Member

@markshannon markshannon commented Apr 8, 2020

Makes no assumptions about the layout of bytecode.
Makes setting frame.f_lineno more robust and flexible.

To improve robustness, the code takes the simple, pragmatic approach: If it is safe to make the jump, then do so. We no longer attempt to decide if it "reasonable" or "sensible", merely if it is safe.
"Safe" in this context, means "won't crash the interpreter".

The increased flexibility is a side effect of this more pragmatic approach.
There are number of test cases where a jump is safe, but we disallowed it. Those cases are now allowed.
A couple of cases are now disallowed. Those involve jumping to unreachable code. Since we cannot compute the exception stack state for unreachable code, it is unsafe to jump to it.

Removing assumptions about the bytecode layout will allow us to enhance the compiler without worrying about breaking this code all the time.

Another point in favour of this PR is that it reduces the code size by about 80 lines.

https://bugs.python.org/issue40228

Copy link
Contributor

Choose a reason for hiding this comment

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

Please put the { on its own line (PEP 7).

Copy link
Contributor

Choose a reason for hiding this comment

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

Please check this call for failure.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please use PyBytes_GET_SIZE() here instead of PyBytes_Size().

Copy link
Contributor

Choose a reason for hiding this comment

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

lines is leaked here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please use PyBytes_GET_SIZE().

Copy link
Contributor

Choose a reason for hiding this comment

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

PyBytes_AS_STRING().

PEP 7 states that no line should be longer than 79 characters.

Copy link
Contributor

Choose a reason for hiding this comment

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

PyBytes_AS_STRING().

@markshannon
Copy link
Member Author

@ZackerySpytz thanks for the review

@markshannon
Copy link
Member Author

Re-basing to force test re-run before merging.

@markshannon markshannon force-pushed the more-robust-frame-setlineno branch from 35a951c to 55daf2c Compare April 29, 2020 15:11
@markshannon markshannon merged commit 5769724 into python:master Apr 29, 2020
@markshannon markshannon deleted the more-robust-frame-setlineno branch April 29, 2020 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants