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

Fix handling of exception edges in backward analysis #4863

Merged

Conversation

msridhar
Copy link
Contributor

@msridhar msridhar commented Aug 4, 2021

Fixes #4861

@msridhar msridhar requested a review from kelloggm August 4, 2021 16:15
a [ LocalVariable ]
~~~~~~~~~
TransferInput#21
After: live variables = a, b
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before the logic change in BackwardAnalysisImpl, the live variable set here would incorrectly be missing b

@msridhar msridhar changed the title Correct handling of exception edges in backward analysis Fix handling of exception edges in backward analysis Aug 4, 2021
@kelloggm
Copy link
Contributor

kelloggm commented Aug 4, 2021

This looks reasonable to me, but I'm not the right person to review it, nor am I sure who is. @mernst, can you either review this change yourself or assign someone else who you think is an appropriate reviewer?

@kelloggm kelloggm requested a review from mernst August 4, 2021 18:29
Copy link
Contributor

@kelloggm kelloggm left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@mernst mernst left a comment

Choose a reason for hiding this comment

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

Thanks!

@mernst mernst merged commit c5886a9 into typetools:master Aug 5, 2021
@msridhar msridhar deleted the backward-analysis-exception-edges branch November 9, 2023 17:42
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.

Incorrect logic in BackwardAnalysisImpl for exceptional successors
3 participants