Skip to content

Conversation

@MathiasVP
Copy link
Contributor

@MathiasVP MathiasVP commented Oct 5, 2023

#14376 fixed a bug where we incorrectly shared representation between certain dataflow nodes.

This fix revealed another bug: the "reverse flow" mechanism (which we have for detecting that something like myMap["abc"] = source(); is a write that modifies myMap) didn't account for the case where the OutNode that represents the result of myMap["abc"] was an instruction instead of an operand.

This bug was hidden until #14376 was merged because these operand and instruction nodes accidentally shared representation.

OutNodes are operands most of the time, but they're instructions when the result of the CallInstruction has multiple uses. One of the cases where this happens after the frontend upgrade is when an assignment (which in this case was a call to an overloaded = operator) happens as an argument to a function call such as foo(m11["abc"] = source());.

@github-actions github-actions bot added the C++ label Oct 5, 2023
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label Oct 5, 2023
@MathiasVP MathiasVP marked this pull request as ready for review October 5, 2023 15:11
@MathiasVP MathiasVP requested a review from a team as a code owner October 5, 2023 15:11
Copy link
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.

LGTM if DCA is happy.

@MathiasVP MathiasVP merged commit b231b1c into github:main Oct 5, 2023
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