Skip to content

Conversation

@jjonescz
Copy link
Member

@jjonescz jjonescz commented Aug 19, 2025

The deconstruction expression has left (the tuple) and right (the expression) sides. But it also carries an invocation info which is of the form leftPlaceholder.Deconstruct(...). Except the receiver (leftPlaceholder) can also be wrapped in a first-class-span conversion (or whatever else can binding of the call do). The problem was that by default, our visitors only visit left and right. But RefSafetyAnalysis then analyzes the invocation and would fail in a Debug.Assert because the receiver wasn't visited.

Fixes #78682.

@jjonescz jjonescz marked this pull request as ready for review August 19, 2025 16:14
@jjonescz jjonescz requested a review from a team as a code owner August 19, 2025 16:14
Copy link
Member

Choose a reason for hiding this comment

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

Other than the assert, what effect does the missed visit have? Are there ref safety issues we're missing reporting?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. At least now, but the assert is there for a reason - we might forget to visit some nodes that could contribute to ref analysis although that's probably unlikely in this case, hence I would be fine with just tracking the visit in DEBUG instead of doing an actual visit if that's what you are getting at.

@jjonescz jjonescz requested a review from a team August 20, 2025 08:06
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (commit 1)

@jcouv jcouv self-assigned this Aug 26, 2025
@jjonescz jjonescz merged commit 05d560e into dotnet:main Aug 29, 2025
25 checks passed
@jjonescz jjonescz deleted the 78682-RefDeconstructionAssert branch August 29, 2025 08:46
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Aug 29, 2025
@akhera99 akhera99 modified the milestones: Next, 18.0 P1, 18.0 P2 Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Assertion in ref analysis of deconstruction with extension and array to span conversion on receiver

4 participants