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 Issue 3281 by merging stores after expression statements #151

Closed
wants to merge 4 commits into from

Conversation

d367wang
Copy link

@d367wang d367wang commented Jul 8, 2020

This PR fixes typetools#3281 by merging stores after expression statements, as follows.

  • When building the CFG, the top-level nodes of all the expression statements are saved in a set.
  • In analyzing phase, check whether the visited node is in the above set while containing both then- and else- stores. If true, merge the two stores.

public List<ReturnNode> getReturnNodes() {
return returnNodes;
}

/**
* Return the top-level nodes of all the expression statements encountered.

Choose a reason for hiding this comment

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

To be consistent, here maybe also use Returns?

Copy link
Author

Choose a reason for hiding this comment

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

Okay, thanks for pointing out!

@wmdietl wmdietl self-assigned this Jul 9, 2020
@d367wang d367wang requested a review from wmdietl July 12, 2020 14:32
@@ -3616,7 +3669,11 @@ public Node visitErroneous(ErroneousTree tree, Void p) {

@Override
public Node visitExpressionStatement(ExpressionStatementTree tree, Void p) {
return scan(tree.getExpression(), p);
Node node = scan(tree.getExpression(), p);
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for exploring this option.
Can you investigate the following alternative: Here, create a local variable and assign non-void expressions to that local variable.
I think that should also cause merging of the two branches and is far less invasive than adding expressionStatementRootNodes, for which I don't really see a good use case.

@d367wang
Copy link
Author

d367wang commented Oct 6, 2020

This PR tries to solve typetools#3281. However, the solution touches the dataflow framework infrastructure by handling special cases in method callTransferFunction in ForwardAnalysisImpl.java, which is a bad design.

@d367wang d367wang closed this Oct 6, 2020
d367wang pushed a commit to d367wang/checker-framework that referenced this pull request Jan 21, 2022
Co-authored-by: Suzanne Millstein <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Michael Ernst <[email protected]>
Co-authored-by: Manu Sridharan <[email protected]>
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.

Dataflow facts from ExpressionStatements
3 participants