Skip to content

C++: Fix asExpr and asIndirectExpr in IR dataflow#10995

Merged
jketema merged 7 commits intogithub:mathiasvp/replace-ast-with-ir-use-usedataflowfrom
MathiasVP:fix-as-expr
Oct 28, 2022
Merged

C++: Fix asExpr and asIndirectExpr in IR dataflow#10995
jketema merged 7 commits intogithub:mathiasvp/replace-ast-with-ir-use-usedataflowfrom
MathiasVP:fix-as-expr

Conversation

@MathiasVP
Copy link
Copy Markdown
Contributor

@MathiasVP MathiasVP commented Oct 26, 2022

This PR contains my ongoing progress on fixing some of the issues I encountered with @jketema while we were trying to fixup the cpp/non-constant-format query in preparation for use-use dataflow.

I'm still not super happy with the PR. I think there are some nodes that we want to have a asExpr result for, but which currently don't have a result because .getConvertedResultExpression() doesn't have a result for some of the instruction nodes we care about (for instance, VariableAddressInstructions). This has been fixed in e2a0d62.

…' to structure conversions between indirect daaflow nodes and expressions.
@MathiasVP MathiasVP requested a review from a team as a code owner October 26, 2022 15:12
@github-actions github-actions Bot added the C++ label Oct 26, 2022
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label Oct 27, 2022
Copy link
Copy Markdown
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

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

Given what we discussed f2f and my current knowledge of this, this looks sensible.

@MathiasVP
Copy link
Copy Markdown
Contributor Author

Merge conflicts fixed. They were as simple as accepting both changes 🎉.

@jketema jketema merged commit fafc0b5 into github:mathiasvp/replace-ast-with-ir-use-usedataflow Oct 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C++ 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