Skip to content

Unified: Fix AST, CFG, and data flow rules for assignments - #22595

Open
asgerf wants to merge 12 commits into
github:mainfrom
asgerf:unified/binary-expr-assignment
Open

asgerf wants to merge 12 commits into
github:mainfrom
asgerf:unified/binary-expr-assignment

Conversation

@asgerf

@asgerf asgerf commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Performs some cross-cutting changes to assignments, ultimately improving data flow:

  • Assignments are now mapped to BinaryExpr in the raw AST schema.
  • The QL classes AssignExpr and CompoundAssignExpr emulate the original AST nodes on the QL side.
  • Updates CFG to support assignments. This was previously blocked on assignments not being subtypes of BinaryExpr.
  • Updates data flow to associate more precise CFG nodes with assignments.
  • Lastly, associates post-update nodes with a more precise CFG node. This required a minor update to the SSA library, hence the many DCA runs. Other languages should not be affected yet, but they can use the same trick when they are ready.

The last point is perhaps worth elaborating on. Most languages have a known issue with false flow in this case

x.f = foo(x)

where the post-update for x on the left-hand side flows into the x on the right-hand side. This fixes the issue by inserting a synthetic read of x after the assignment and essentially using that as the post-update node.

This does require a bit of follow-up work in case x is re-assigned inside the RHS expression, but since Swift assignments are not expressions, this can't happen in Swift, and we can't even write a test case for it. But the fix would be to block the post-update node if the two reads don't have the same SSA definition.

@asgerf
asgerf force-pushed the unified/binary-expr-assignment branch 2 times, most recently from 769cc55 to ee8bcd7 Compare September 17, 2026 11:16
@asgerf asgerf added the no-change-note-required This PR does not need a change note label Sep 17, 2026
@asgerf
asgerf force-pushed the unified/binary-expr-assignment branch from 8feb94d to e167281 Compare September 17, 2026 11:33
@asgerf
asgerf marked this pull request as ready for review September 17, 2026 11:38
@asgerf
asgerf requested review from a team as code owners September 17, 2026 11:38
Copilot AI balanced review requested due to automatic review settings September 17, 2026 11:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@asgerf

asgerf commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants