Deconstruction into dynamic array and dynamic indexer #13724
Conversation
jcouv
force-pushed
the
dynamic-tuple
branch
from
fde237c
to
c994d12
Compare
|
@AlekseyTs @dotnet/roslyn-compiler for review. Thank |
| comp.VerifyDiagnostics(); | ||
| // No runtime failure (System.ArrayTypeMismatchException: Attempted to access an element as a type incompatible with the array.) | ||
| // because of the special handling for dynamic in LocalRewriter.TransformCompoundAssignmentLHS | ||
| } |
There was a problem hiding this comment.
Was this just a missing test or does the test hit the rewriter changes?
There was a problem hiding this comment.
This test hits the change I made. Prior to the fix, this test would produce a runtime exception.
There was a problem hiding this comment.
Ignore my last comment. This test was not crashing.
But we were missing a test that verifies execution of compound assignment with dynamic array.
|
LGTM |
| var rewrittenRight = (BoundExpression)Visit(assignment.Right); | ||
|
|
||
| var loweredAssignment = MakeAssignmentOperator(assignment.Syntax, rewrittenLeft, rewrittenRight, assignment.Type, | ||
| used: true, isChecked: false, isCompoundAssignment: false); |
There was a problem hiding this comment.
isChecked: false [](start = 56, length = 16)
What difference does this parameter make?
There was a problem hiding this comment.
I'm not sure. I'm using the same option that the previous code was using. The local rewriter's VisitAssignmentOperator is implicitly setting isChecked to false (by omission) when it calls MakeDynamicSetIndex or MakeDynamicSetMember.
|
LGTM |
|
Thanks! Going ahead with merge. |
The deconstruction lowering uses
TransformCompoundAssignmentLHSto capture the side-effects of left-hand-side variables and get symbols back for writing into those references. But we were always passingisDynamicAssignmentasfalse.It turns out that is a bug and I added some tests that hit that case. Before this fix, those tests produce code that fails at runtime with
System.ArrayTypeMismatchException: Attempted to access an element as a type incompatible with the array.In this process, I found out that lowering of some of the assignment steps (that are part of the deconstruction) would fail because they would would not switch on the proper
KindinVisitAssignmentOperator. It would switch on the kind of the unlowered left (ie the kind of the placeholder), instead of the proper kindDynamicIndexerAccess.After this fix, lowering of the assignment step directly makes lowered assignment nodes with
MakeAssignmentOperatorinstead of callingVisitExpressionon the un-lowered assignment node.Fixes #12398
@AlekseyTs @dotnet/roslyn-compiler for review.