Skip to content

Fix GetDeconstructionInfo on converted deconstruction assignment#82324

Merged
jcouv merged 4 commits intodotnet:mainfrom
jcouv:deconstruction-info
Feb 14, 2026
Merged

Fix GetDeconstructionInfo on converted deconstruction assignment#82324
jcouv merged 4 commits intodotnet:mainfrom
jcouv:deconstruction-info

Conversation

@jcouv
Copy link
Copy Markdown
Member

@jcouv jcouv commented Feb 6, 2026

Fixes #68979

@jcouv jcouv self-assigned this Feb 6, 2026
@jcouv jcouv marked this pull request as ready for review February 10, 2026 22:40
@jcouv jcouv requested a review from a team as a code owner February 10, 2026 22:40
var boundDeconstruction = GetUpperBoundNode(node) as BoundDeconstructionAssignmentOperator;
if (boundDeconstruction is null)
var upperNode = GetUpperBoundNode(node);
while (upperNode is BoundConversion conversion)
Copy link
Copy Markdown
Contributor

@AlekseyTs AlekseyTs Feb 11, 2026

Choose a reason for hiding this comment

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

while (upperNode is BoundConversion conversion)

It is not obvious to me that this is the right thing to do. Could you provide more details about the tree shape that we are trying to accommodate with this change? #Closed

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.

Would requesting the "lower" bound node work better?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Would requesting the "lower" bound node work better?
Yes, I think that's better. Thanks

Could you provide more details about the tree shape that we are trying to accommodate with this change?

Here's the shape of the tree:
The bound node for the RHS assignment syntax is a conversion chain with a deconstructionAssignmentOperator.

Here's an extract:

│         └─assignmentOperator
│           ├─left
│           │ └─local
│           │   ├─localSymbol: c1
│           │   ├─declarationKind: None
│           │   ├─constantValueOpt
│           │   ├─isNullableUnknown: False
│           │   └─type: C
│           ├─right
│           │ └─conversion
│           │   ├─operand
│           │   │ └─conversion
│           │   │   ├─operand
│           │   │   │ └─deconstructionAssignmentOperator

Here's the full:

nonConstructorMethodBody
├─blockBody
│ └─block
│   ├─locals: {c1, c2}
│   ├─hasUnsafeModifier: False
│   ├─instrumentation
│   └─statements
│     ├─localDeclaration
│     │ ├─localSymbol: c1
│     │ ├─declaredTypeOpt
│     │ │ └─typeExpression
│     │ │   ├─typeWithAnnotations: C
│     │ │   └─type: C
│     │ ├─initializerOpt
│     │ │ └─objectCreationExpression
│     │ │   ├─constructor: C.C()
│     │ │   ├─constructorsGroup: {C.C()}
│     │ │   ├─arguments
│     │ │   ├─argumentNamesOpt: (null)
│     │ │   ├─argumentRefKindsOpt: (null)
│     │ │   ├─expanded: False
│     │ │   ├─argsToParamsOpt: (null)
│     │ │   ├─defaultArguments: Microsoft.CodeAnalysis.BitVector
│     │ │   ├─constantValueOpt
│     │ │   ├─initializerExpressionOpt
│     │ │   ├─wasTargetTyped: False
│     │ │   └─type: C
│     │ ├─argumentsOpt
│     │ └─inferredType: True
│     ├─localDeclaration
│     │ ├─localSymbol: c2
│     │ ├─declaredTypeOpt
│     │ │ └─typeExpression
│     │ │   ├─typeWithAnnotations: C
│     │ │   └─type: C
│     │ ├─initializerOpt
│     │ │ └─objectCreationExpression
│     │ │   ├─constructor: C.C()
│     │ │   ├─constructorsGroup: {C.C()}
│     │ │   ├─arguments
│     │ │   ├─argumentNamesOpt: (null)
│     │ │   ├─argumentRefKindsOpt: (null)
│     │ │   ├─expanded: False
│     │ │   ├─argsToParamsOpt: (null)
│     │ │   ├─defaultArguments: Microsoft.CodeAnalysis.BitVector
│     │ │   ├─constantValueOpt
│     │ │   ├─initializerExpressionOpt
│     │ │   ├─wasTargetTyped: False
│     │ │   └─type: C
│     │ ├─argumentsOpt
│     │ └─inferredType: True
│     └─expressionStatement
│       └─expression
│         └─assignmentOperator
│           ├─left
│           │ └─local
│           │   ├─localSymbol: c1
│           │   ├─declarationKind: None
│           │   ├─constantValueOpt
│           │   ├─isNullableUnknown: False
│           │   └─type: C
│           ├─right
│           │ └─conversion
│           │   ├─operand
│           │   │ └─conversion
│           │   │   ├─operand
│           │   │   │ └─deconstructionAssignmentOperator
│           │   │   │   ├─left
│           │   │   │   │ └─convertedTupleLiteral
│           │   │   │   │   ├─sourceTuple
│           │   │   │   │   │ └─tupleLiteral
│           │   │   │   │   │   ├─arguments
│           │   │   │   │   │   │ ├─discardExpression
│           │   │   │   │   │   │ │ ├─nullableAnnotation: Oblivious
│           │   │   │   │   │   │ │ ├─isInferred: True
│           │   │   │   │   │   │ │ └─type: int
│           │   │   │   │   │   │ └─discardExpression
│           │   │   │   │   │   │   ├─nullableAnnotation: Oblivious
│           │   │   │   │   │   │   ├─isInferred: True
│           │   │   │   │   │   │   └─type: string
│           │   │   │   │   │   ├─argumentNamesOpt: (null)
│           │   │   │   │   │   ├─inferredNamesOpt: (null)
│           │   │   │   │   │   └─type: (int, string)
│           │   │   │   │   ├─wasTargetTyped: False
│           │   │   │   │   ├─arguments
│           │   │   │   │   │ ├─discardExpression
│           │   │   │   │   │ │ ├─nullableAnnotation: Oblivious
│           │   │   │   │   │ │ ├─isInferred: True
│           │   │   │   │   │ │ └─type: int
│           │   │   │   │   │ └─discardExpression
│           │   │   │   │   │   ├─nullableAnnotation: Oblivious
│           │   │   │   │   │   ├─isInferred: True
│           │   │   │   │   │   └─type: string
│           │   │   │   │   ├─argumentNamesOpt: (null)
│           │   │   │   │   ├─inferredNamesOpt: (null)
│           │   │   │   │   └─type: (int, string)
│           │   │   │   ├─right
│           │   │   │   │ └─conversion
│           │   │   │   │   ├─operand
│           │   │   │   │   │ └─local
│           │   │   │   │   │   ├─localSymbol: c2
│           │   │   │   │   │   ├─declarationKind: None
│           │   │   │   │   │   ├─constantValueOpt
│           │   │   │   │   │   ├─isNullableUnknown: False
│           │   │   │   │   │   └─type: C
│           │   │   │   │   ├─conversion: Deconstruction
│           │   │   │   │   ├─isBaseConversion: False
│           │   │   │   │   ├─@checked: False
│           │   │   │   │   ├─explicitCastInCode: False
│           │   │   │   │   ├─constantValueOpt
│           │   │   │   │   ├─conversionGroupOpt
│           │   │   │   │   ├─inConversionGroupFlags: Unspecified
│           │   │   │   │   └─type: (int, string)
│           │   │   │   ├─isUsed: True
│           │   │   │   └─type: (int, string)
│           │   │   ├─conversion: Identity
│           │   │   ├─isBaseConversion: False
│           │   │   ├─@checked: False
│           │   │   ├─explicitCastInCode: False
│           │   │   ├─constantValueOpt
│           │   │   ├─conversionGroupOpt: Microsoft.CodeAnalysis.CSharp.ConversionGroup
│           │   │   ├─inConversionGroupFlags: UserDefinedFromConversion
│           │   │   └─type: (int i, string s)
│           │   ├─conversion: ImplicitUserDefined
│           │   ├─isBaseConversion: False
│           │   ├─@checked: False
│           │   ├─explicitCastInCode: False
│           │   ├─constantValueOpt
│           │   ├─conversionGroupOpt: Microsoft.CodeAnalysis.CSharp.ConversionGroup
│           │   ├─inConversionGroupFlags: UserDefinedOperator
│           │   └─type: C
│           └─type: C
└─expressionBody

@AlekseyTs
Copy link
Copy Markdown
Contributor

Done with review pass (commit 2)

@jcouv jcouv changed the title GetDeconstructionInfo should dig through conversions GetDeconstructionInfo should look through conversions Feb 11, 2026
@jcouv jcouv requested a review from AlekseyTs February 11, 2026 21:03
{
var boundDeconstruction = GetUpperBoundNode(node) as BoundDeconstructionAssignmentOperator;
if (boundDeconstruction is null)
var upperNode = GetLowerBoundNode(node);
Copy link
Copy Markdown
Contributor

@AlekseyTs AlekseyTs Feb 12, 2026

Choose a reason for hiding this comment

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

upperNode

It looks like the name of the local no longer reflects its meaning. #Closed

@AlekseyTs
Copy link
Copy Markdown
Contributor

Done with review pass (commit 3)

@jcouv jcouv requested review from a team and AlekseyTs February 12, 2026 03:40
Copy link
Copy Markdown
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (commit 4)

@AlekseyTs
Copy link
Copy Markdown
Contributor

@jcouv Consider adjusting the title to reflect the nature of the change

@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented Feb 12, 2026

@dotnet/roslyn-compiler for another review. Thanks

@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented Feb 12, 2026

@jcouv Consider adjusting the title to reflect the nature of the change

I think "GetDeconstructionInfo should look through conversions" is still applicable, since that's the problem being solved.

@AlekseyTs
Copy link
Copy Markdown
Contributor

I think "GetDeconstructionInfo should look through conversions" is still applicable, since that's the problem being solved.

I don't think I agree. The problem being solved is to look at the right node.

@AlekseyTs
Copy link
Copy Markdown
Contributor

And the change, as it stands right now, is not looking through conversions

@jcouv jcouv changed the title GetDeconstructionInfo should look through conversions Fix GetDeconstructionInfo on converted deconstruction assignment Feb 13, 2026
@jcouv jcouv requested a review from a team February 13, 2026 18:55
@jcouv jcouv merged commit d3884fa into dotnet:main Feb 14, 2026
24 checks passed
@jcouv jcouv deleted the deconstruction-info branch February 14, 2026 09:44
@dotnet-policy-service dotnet-policy-service Bot added this to the Next milestone Feb 14, 2026
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.

'GetDeconstructionInfo' doesn't return Deconstruct method symbol

4 participants