Skip to content
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-107901: make compiler inline basic blocks with no line number and no fallthrough #114750

Merged
merged 4 commits into from
Feb 2, 2024

Conversation

iritkatriel
Copy link
Member

@iritkatriel iritkatriel commented Jan 30, 2024

This pattern shows up in code like this:

def f():         
    for e in seq:
        try:
            X = 3
        except OSError:
            try:
                if C3:
                    X = 4
            except OSError:
                pass
    return 42

We have a block with no line number, and two predecessors, which contains a jump to another block that has no line number. The two-predecessor block is not an exit block so it is not duplicated. As a result, the end block remains with no line number, even if it needs one. This change inlines the first block to each of its predecessors, so that the jump can get a line number.

@markshannon
Copy link
Member

Two questions.

  • Does this fix handle triply nested try-excepts?
  • Is it feasible to add a test for this?

@iritkatriel
Copy link
Member Author

  • Does this fix handle triply nested try-excepts?

It does. The issue is not the nesting, but the multiple paths leading to the end of the except block. It needs to be nested in another except because otherwise the blocks get reordered and this impacts the situation. But with nesting all the blocks are cold, so they remain in this order. More nesting doesn't make a difference.

  • Is it feasible to add a test for this?

I'll try to add a unit test.

This PR makes the build pass with the assertion that all eval break checks have line numbers (there are 3 more test failures then, but at least we get through the build).

Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

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

Looks good

@iritkatriel iritkatriel merged commit 2091fb2 into python:main Feb 2, 2024
34 checks passed
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
fsc-eriker pushed a commit to fsc-eriker/cpython that referenced this pull request Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants