Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Left to Right semantics with fields and arrays in async #42807

Merged
merged 12 commits into from
Apr 17, 2020
Merged
55 changes: 50 additions & 5 deletions src/Compilers/CSharp/Portable/Lowering/SpillSequenceSpiller.cs
Original file line number Diff line number Diff line change
Expand Up @@ -677,17 +677,15 @@ public override BoundNode VisitAssignmentOperator(BoundAssignmentOperator node)
if (field.FieldSymbol.IsStatic) break;

// instance fields are directly assignable, but receiver is pushed, so need to spill that.
var receiver = VisitExpression(ref leftBuilder, field.ReceiverOpt);
receiver = Spill(builder, receiver, field.FieldSymbol.ContainingType.IsValueType ? RefKind.Ref : RefKind.None);
left = field.Update(receiver, field.FieldSymbol, field.ConstantValueOpt, field.ResultKind, field.Type);
left = fieldWithSpilledReceiver(field, ref leftBuilder, isAssignmentTarget: true);
break;

case BoundKind.ArrayAccess:
var arrayAccess = (BoundArrayAccess)left;
// array and indices are pushed on stack so need to spill that
var expression = VisitExpression(ref leftBuilder, arrayAccess.Expression);
expression = Spill(builder, expression, RefKind.None);
var indices = this.VisitExpressionList(ref builder, arrayAccess.Indices, forceSpill: true);
expression = Spill(leftBuilder, expression, RefKind.None);
var indices = this.VisitExpressionList(ref leftBuilder, arrayAccess.Indices, forceSpill: true);
left = arrayAccess.Update(expression, indices, arrayAccess.Type);
break;

Expand All @@ -710,6 +708,53 @@ public override BoundNode VisitAssignmentOperator(BoundAssignmentOperator node)
}

return UpdateExpression(builder, node.Update(left, right, node.IsRef, node.Type));

BoundExpression fieldWithSpilledReceiver(BoundFieldAccess field, ref BoundSpillSequenceBuilder leftBuilder, bool isAssignmentTarget)
{
BoundExpression receiver;
var generateDummyFieldAccess = false;
if (field.FieldSymbol.ContainingType.IsReferenceType)
{
// a reference type can always live across await so Spill using leftBuilder
receiver = Spill(leftBuilder, VisitExpression(ref leftBuilder, field.ReceiverOpt));

// dummy field access to trigger NRE
// a.b = c will trigger a NRE if a is null on assignment,
// but a.b.c = d will trigger a NRE if a is null before evaluating d
// so check whether we assign to the field directly
generateDummyFieldAccess = !isAssignmentTarget;
}
else if (field.ReceiverOpt is BoundArrayAccess arrayAccess)
{
// an arrayAccess returns a ref so can only be called after the await, but spill expression and indices
var expression = VisitExpression(ref leftBuilder, arrayAccess.Expression);
expression = Spill(leftBuilder, expression, RefKind.None);
var indices = this.VisitExpressionList(ref leftBuilder, arrayAccess.Indices, forceSpill: true);
receiver = arrayAccess.Update(expression, indices, arrayAccess.Type);
// dummy array access to trigger IndexOutRangeException or NRE
// we only need this if the array access is a receiver since
// a[0] = b triggers a NRE/IORE on assignment
333fred marked this conversation as resolved.
Show resolved Hide resolved
// but a[0].b = c triggers an NRE/IORE before evaluating c
Spill(leftBuilder, receiver, sideEffectsOnly: true);
333fred marked this conversation as resolved.
Show resolved Hide resolved
333fred marked this conversation as resolved.
Show resolved Hide resolved
}
else if (field.ReceiverOpt is BoundFieldAccess receiverField)
{
receiver = fieldWithSpilledReceiver(receiverField, ref leftBuilder, isAssignmentTarget: false);
}
else
{
receiver = Spill(leftBuilder, VisitExpression(ref leftBuilder, field.ReceiverOpt), RefKind.Ref);
333fred marked this conversation as resolved.
Show resolved Hide resolved
}

field = field.Update(receiver, field.FieldSymbol, field.ConstantValueOpt, field.ResultKind, field.Type);

if (generateDummyFieldAccess)
{
Spill(leftBuilder, field, sideEffectsOnly: true);
}

return field;
}
}

public override BoundNode VisitBadExpression(BoundBadExpression node)
Expand Down
Loading