Skip to content

Commit 05dd8e6

Browse files
AlekseyTsAlekseyTs
authored
Fix code gen for some compound assignment scenarios involving extension properties. (#79339)
1 parent e11f53e commit 05dd8e6

File tree

7 files changed

+5253
-630
lines changed

7 files changed

+5253
-630
lines changed

‎src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CompoundAssignmentOperator.cs

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,27 @@ BoundExpression rewriteAssignment(BoundExpression leftRead, BoundExpression righ
226226
}
227227

228228
Debug.Assert(TypeSymbol.Equals(transformedLHS.Type, node.Left.Type, TypeCompareKind.AllIgnoreOptions));
229+
230+
if (IsNewExtensionMemberAccessWithByValPossiblyStructReceiver(transformedLHS))
231+
{
232+
// We need to create a tree that ensures that receiver of 'set' is evaluated after the binary operation
233+
BoundLocal binaryResult = _factory.StoreToTemp(opFinal, out BoundAssignmentOperator assignmentToTemp, refKind: RefKind.None);
234+
BoundExpression assignment = MakeAssignmentOperator(syntax, transformedLHS, binaryResult, used: used, isChecked: isChecked, isCompoundAssignment: true);
235+
Debug.Assert(assignment.Type is { });
236+
return new BoundSequence(syntax, [binaryResult.LocalSymbol], [assignmentToTemp], assignment, assignment.Type);
237+
}
238+
229239
return MakeAssignmentOperator(syntax, transformedLHS, opFinal, used: used, isChecked: isChecked, isCompoundAssignment: true);
230240
}
231241
}
232242

233-
private BoundExpression? TransformPropertyOrEventReceiver(Symbol propertyOrEvent, BoundExpression? receiverOpt, bool isRegularCompoundAssignment, ArrayBuilder<BoundExpression> stores, ArrayBuilder<LocalSymbol> temps)
243+
private static bool IsNewExtensionMemberAccessWithByValPossiblyStructReceiver(BoundExpression transformedLHS)
244+
{
245+
return transformedLHS is BoundPropertyAccess { PropertySymbol: { } property } && property.GetIsNewExtensionMember() &&
246+
property.ContainingType.ExtensionParameter is { RefKind: RefKind.None, Type.IsReferenceType: false };
247+
}
248+
249+
private BoundExpression? TransformPropertyOrEventReceiver(Symbol propertyOrEvent, BoundExpression? receiverOpt, ArrayBuilder<BoundExpression> stores, ArrayBuilder<LocalSymbol> temps)
234250
{
235251
Debug.Assert(propertyOrEvent.Kind == SymbolKind.Property || propertyOrEvent.Kind == SymbolKind.Event);
236252

@@ -272,7 +288,9 @@ BoundExpression rewriteAssignment(BoundExpression leftRead, BoundExpression righ
272288
// SPEC VIOLATION: However, for compatibility with Dev12 we will continue treating all generic type parameters, constrained or not,
273289
// SPEC VIOLATION: as value types.
274290
Debug.Assert(rewrittenReceiver.Type is { });
275-
var variableRepresentsLocation = rewrittenReceiver.Type.IsValueType || rewrittenReceiver.Type.Kind == SymbolKind.TypeParameter;
291+
var variableRepresentsLocation = propertyOrEvent.GetIsNewExtensionMember() ?
292+
!rewrittenReceiver.Type.IsReferenceType :
293+
(rewrittenReceiver.Type.IsValueType || rewrittenReceiver.Type.Kind == SymbolKind.TypeParameter);
276294

277295
var receiverTemp = _factory.StoreToTemp(
278296
rewrittenReceiver,
@@ -286,9 +304,10 @@ BoundExpression rewriteAssignment(BoundExpression leftRead, BoundExpression righ
286304
stackLocalsOpt: null));
287305
temps.Add(receiverTemp.LocalSymbol);
288306

289-
if (!isRegularCompoundAssignment &&
290-
receiverTemp.LocalSymbol.IsRef &&
291-
CodeGenerator.IsPossibleReferenceTypeReceiverOfConstrainedCall(receiverTemp) &&
307+
if (receiverTemp.LocalSymbol.IsRef &&
308+
(propertyOrEvent.GetIsNewExtensionMember() ?
309+
!receiverTemp.Type.IsValueType :
310+
CodeGenerator.IsPossibleReferenceTypeReceiverOfConstrainedCall(receiverTemp)) &&
292311
!CodeGenerator.ReceiverIsKnownToReferToTempIfReferenceType(receiverTemp))
293312
{
294313
BoundAssignmentOperator? extraRefInitialization;
@@ -678,7 +697,7 @@ private BoundExpression TransformCompoundAssignmentLHS(BoundExpression originalL
678697
{
679698
// This is a temporary object that will be rewritten away before the lowering completes.
680699
return propertyAccess.Update(TransformPropertyOrEventReceiver(propertyAccess.PropertySymbol, propertyAccess.ReceiverOpt,
681-
isRegularCompoundAssignment, stores, temps),
700+
stores, temps),
682701
propertyAccess.InitialBindingReceiverIsSubjectToCloning, propertyAccess.PropertySymbol, propertyAccess.AutoPropertyAccessorKind, propertyAccess.ResultKind, propertyAccess.Type);
683702
}
684703
}
@@ -804,10 +823,9 @@ private BoundExpression TransformCompoundAssignmentLHS(BoundExpression originalL
804823

805824
if (eventAccess.EventSymbol.IsWindowsRuntimeEvent)
806825
{
807-
Debug.Assert(!isRegularCompoundAssignment);
808826
// This is a temporary object that will be rewritten away before the lowering completes.
809827
return eventAccess.Update(TransformPropertyOrEventReceiver(eventAccess.EventSymbol, eventAccess.ReceiverOpt,
810-
isRegularCompoundAssignment, stores, temps),
828+
stores, temps),
811829
eventAccess.EventSymbol, eventAccess.IsUsableAsField, eventAccess.ResultKind, eventAccess.Type);
812830
}
813831

‎src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_NullCoalescingAssignmentOperator.cs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,20 @@ BoundExpression rewriteNullCoalscingAssignmentStandard()
3939
// See CodeGenNullCoalescingAssignmentTests.CoalescingAssignment_DynamicRuntimeCastFailure, which will fail if
4040
// isCompoundAssignment is set to true. It will fail to throw a runtime binder cast exception.
4141
Debug.Assert(TypeSymbol.Equals(transformedLHS.Type, node.LeftOperand.Type, TypeCompareKind.AllIgnoreOptions));
42-
BoundExpression assignment = MakeAssignmentOperator(syntax, transformedLHS, loweredRight, used: true, isChecked: false, isCompoundAssignment: false);
42+
BoundExpression assignment;
43+
44+
if (IsNewExtensionMemberAccessWithByValPossiblyStructReceiver(transformedLHS))
45+
{
46+
// We need to create a tree that ensures that receiver of 'set' is evaluated after the right hand side value
47+
BoundLocal rightResult = _factory.StoreToTemp(loweredRight, out BoundAssignmentOperator assignmentToTemp, refKind: RefKind.None);
48+
assignment = MakeAssignmentOperator(syntax, transformedLHS, rightResult, used: true, isChecked: false, isCompoundAssignment: false);
49+
Debug.Assert(assignment.Type is { });
50+
assignment = new BoundSequence(syntax, [rightResult.LocalSymbol], [assignmentToTemp], assignment, assignment.Type);
51+
}
52+
else
53+
{
54+
assignment = MakeAssignmentOperator(syntax, transformedLHS, loweredRight, used: true, isChecked: false, isCompoundAssignment: false);
55+
}
4356

4457
// lhsRead ?? (transformedLHS = loweredRight)
4558
var leftPlaceholder = new BoundValuePlaceholder(lhsRead.Syntax, lhsRead.Type);

‎src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_UnaryOperator.cs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,7 @@ public BoundExpression VisitBuiltInOrStaticIncrementOperator(BoundIncrementOpera
621621
//
622622
if (isIndirectOrInstanceField(transformedLHS))
623623
{
624+
Debug.Assert(transformedLHS is not BoundPropertyAccess);
624625
return rewriteWithRefOperand(isPrefix, isChecked, tempSymbols, tempInitializers, syntax, transformedLHS, boundTemp, newValue);
625626
}
626627
else
@@ -660,16 +661,27 @@ BoundExpression rewriteWithNotRefOperand(
660661

661662
// prefix: temp = (X)(T.Increment((T)operand))); operand = temp;
662663
// postfix: temp = operand; operand = (X)(T.Increment((T)temp)));
663-
ImmutableArray<BoundExpression> assignments = ImmutableArray.Create<BoundExpression>(
664-
MakeAssignmentOperator(syntax, boundTemp, isPrefix ? newValue : MakeRValue(transformedLHS), used: false, isChecked: isChecked, isCompoundAssignment: false),
665-
MakeAssignmentOperator(syntax, transformedLHS, isPrefix ? boundTemp : newValue, used: false, isChecked: isChecked, isCompoundAssignment: false));
664+
tempInitializers.Add(MakeAssignmentOperator(syntax, boundTemp, isPrefix ? newValue : MakeRValue(transformedLHS), used: false, isChecked: isChecked, isCompoundAssignment: false));
665+
666+
if (!isPrefix && IsNewExtensionMemberAccessWithByValPossiblyStructReceiver(transformedLHS))
667+
{
668+
// We need to create a tree that ensures that receiver of 'set' is evaluated after the increment operation
669+
BoundLocal incrementResult = _factory.StoreToTemp(newValue, out BoundAssignmentOperator assignmentToTemp, refKind: RefKind.None);
670+
tempSymbols.Add(incrementResult.LocalSymbol);
671+
tempInitializers.Add(assignmentToTemp);
672+
tempInitializers.Add(MakeAssignmentOperator(syntax, transformedLHS, incrementResult, used: false, isChecked: isChecked, isCompoundAssignment: false));
673+
}
674+
else
675+
{
676+
tempInitializers.Add(MakeAssignmentOperator(syntax, transformedLHS, isPrefix ? boundTemp : newValue, used: false, isChecked: isChecked, isCompoundAssignment: false));
677+
}
666678

667679
// prefix: Seq( operand initializers; temp = (T)(operand + 1); operand = temp; result: temp)
668680
// postfix: Seq( operand initializers; temp = operand; operand = (T)(temp + 1); result: temp)
669681
return new BoundSequence(
670682
syntax: syntax,
671683
locals: tempSymbols.ToImmutableAndFree(),
672-
sideEffects: tempInitializers.ToImmutableAndFree().Concat(assignments),
684+
sideEffects: tempInitializers.ToImmutableAndFree(),
673685
value: boundTemp,
674686
type: boundTemp.Type);
675687
}

‎src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenOperators.cs

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5228,41 +5228,43 @@ public static void Repro2(T arg)
52285228
1");
52295229
compilation.VerifyIL("test<T>.Repro1(T)", @"
52305230
{
5231-
// Code size 88 (0x58)
5231+
// Code size 86 (0x56)
52325232
.maxstack 4
5233-
.locals init (T& V_0)
5233+
.locals init (T& V_0,
5234+
T V_1)
52345235
IL_0000: ldarg.0
52355236
IL_0001: box ""T""
52365237
IL_0006: dup
52375238
IL_0007: ldfld ""int c0.x""
52385239
IL_000c: ldc.i4.1
52395240
IL_000d: add
52405241
IL_000e: stfld ""int c0.x""
5241-
IL_0013: ldarga.s V_0
5242-
IL_0015: stloc.0
5243-
IL_0016: ldloc.0
5244-
IL_0017: ldobj ""T""
5245-
IL_001c: box ""T""
5246-
IL_0021: ldloc.0
5247-
IL_0022: constrained. ""T""
5248-
IL_0028: callvirt ""int c0.P1.get""
5249-
IL_002d: ldc.i4.1
5250-
IL_002e: add
5251-
IL_002f: callvirt ""void c0.P1.set""
5252-
IL_0034: ldarga.s V_0
5253-
IL_0036: stloc.0
5254-
IL_0037: ldloc.0
5255-
IL_0038: ldobj ""T""
5256-
IL_003d: box ""T""
5242+
IL_0013: ldarg.0
5243+
IL_0014: stloc.1
5244+
IL_0015: ldloca.s V_1
5245+
IL_0017: stloc.0
5246+
IL_0018: ldloc.0
5247+
IL_0019: ldloc.0
5248+
IL_001a: constrained. ""T""
5249+
IL_0020: callvirt ""int c0.P1.get""
5250+
IL_0025: ldc.i4.1
5251+
IL_0026: add
5252+
IL_0027: constrained. ""T""
5253+
IL_002d: callvirt ""void c0.P1.set""
5254+
IL_0032: ldarga.s V_0
5255+
IL_0034: stloc.0
5256+
IL_0035: ldloc.0
5257+
IL_0036: ldobj ""T""
5258+
IL_003b: box ""T""
5259+
IL_0040: ldc.i4.1
5260+
IL_0041: ldloc.0
52575261
IL_0042: ldc.i4.1
5258-
IL_0043: ldloc.0
5259-
IL_0044: ldc.i4.1
5260-
IL_0045: constrained. ""T""
5261-
IL_004b: callvirt ""int c0.this[int].get""
5262-
IL_0050: ldc.i4.1
5263-
IL_0051: add
5264-
IL_0052: callvirt ""void c0.this[int].set""
5265-
IL_0057: ret
5262+
IL_0043: constrained. ""T""
5263+
IL_0049: callvirt ""int c0.this[int].get""
5264+
IL_004e: ldc.i4.1
5265+
IL_004f: add
5266+
IL_0050: callvirt ""void c0.this[int].set""
5267+
IL_0055: ret
52665268
}
52675269
").VerifyIL("test<T>.Repro2(T)", @"
52685270
{

0 commit comments

Comments
 (0)