Skip to content

Commit 8b3fbe4

Browse files
CopilotCyrusNajmabadi
Copilot
and
CyrusNajmabadi
authored
Use constant indices when populating Lists and arrays in collection expressions (#80999)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com> Co-authored-by: Cyrus Najmabadi <cyrus.najmabadi@gmail.com>
1 parent 68873c8 commit 8b3fbe4

File tree

2 files changed

+396
-274
lines changed

2 files changed

+396
-274
lines changed

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

Lines changed: 90 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -831,12 +831,19 @@ private BoundExpression CreateAndPopulateArray(BoundCollectionExpression node, A
831831

832832
RewriteCollectionExpressionElementsIntoTemporaries(elements, numberIncludingLastSpread, localsBuilder, sideEffects);
833833

834-
// int index = 0;
835-
BoundLocal indexTemp = _factory.StoreToTemp(
836-
_factory.Literal(0),
837-
out assignmentToTemp);
838-
localsBuilder.Add(indexTemp);
839-
sideEffects.Add(assignmentToTemp);
834+
// indexTemp is null when we can use constant compile-time indices.
835+
// indexTemp is non-null when we need a runtime-tracked index variable (for spread elements).
836+
BoundLocal? indexTemp = null;
837+
838+
if (numberIncludingLastSpread != 0)
839+
{
840+
// int index = 0;
841+
indexTemp = _factory.StoreToTemp(
842+
_factory.Literal(0),
843+
out assignmentToTemp);
844+
localsBuilder.Add(indexTemp);
845+
sideEffects.Add(assignmentToTemp);
846+
}
840847

841848
// ElementType[] array = new ElementType[N + s1.Length + ...];
842849
BoundLocal arrayTemp = _factory.StoreToTemp(
@@ -848,6 +855,7 @@ private BoundExpression CreateAndPopulateArray(BoundCollectionExpression node, A
848855
localsBuilder.Add(arrayTemp);
849856
sideEffects.Add(assignmentToTemp);
850857

858+
int currentElementIndex = 0;
851859
AddCollectionExpressionElements(
852860
elements,
853861
arrayTemp,
@@ -857,33 +865,50 @@ private BoundExpression CreateAndPopulateArray(BoundCollectionExpression node, A
857865
addElement: (ArrayBuilder<BoundExpression> expressions, BoundExpression arrayTemp, BoundExpression rewrittenValue, bool isLastElement) =>
858866
{
859867
Debug.Assert(arrayTemp.Type is ArrayTypeSymbol);
860-
Debug.Assert(indexTemp.Type is { SpecialType: SpecialType.System_Int32 });
861868

862869
var expressionSyntax = rewrittenValue.Syntax;
863870
var elementType = ((ArrayTypeSymbol)arrayTemp.Type).ElementType;
864871

865-
// array[index] = element;
866-
expressions.Add(
867-
new BoundAssignmentOperator(
868-
expressionSyntax,
869-
_factory.ArrayAccess(arrayTemp, indexTemp),
870-
rewrittenValue,
871-
isRef: false,
872-
elementType));
873-
if (!isLastElement)
872+
if (indexTemp is null)
874873
{
875-
// index = index + 1;
874+
// array[0] = element; array[1] = element; etc.
876875
expressions.Add(
877876
new BoundAssignmentOperator(
878877
expressionSyntax,
879-
indexTemp,
880-
_factory.Binary(BinaryOperatorKind.Addition, indexTemp.Type, indexTemp, _factory.Literal(1)),
878+
_factory.ArrayAccess(arrayTemp, _factory.Literal(currentElementIndex)),
879+
rewrittenValue,
881880
isRef: false,
882-
indexTemp.Type));
881+
elementType));
882+
currentElementIndex++;
883+
}
884+
else
885+
{
886+
// array[index] = element;
887+
expressions.Add(
888+
new BoundAssignmentOperator(
889+
expressionSyntax,
890+
_factory.ArrayAccess(arrayTemp, indexTemp),
891+
rewrittenValue,
892+
isRef: false,
893+
elementType));
894+
if (!isLastElement)
895+
{
896+
// index = index + 1;
897+
expressions.Add(
898+
new BoundAssignmentOperator(
899+
expressionSyntax,
900+
indexTemp,
901+
_factory.Binary(BinaryOperatorKind.Addition, indexTemp.Type, indexTemp, _factory.Literal(1)),
902+
isRef: false,
903+
indexTemp.Type));
904+
}
883905
}
884906
},
885907
tryOptimizeSpreadElement: (ArrayBuilder<BoundExpression> sideEffects, BoundExpression arrayTemp, BoundCollectionExpressionSpreadElement spreadElement, BoundExpression rewrittenSpreadOperand) =>
886908
{
909+
// When we have spreads, we always need a runtime-tracked index variable.
910+
Debug.Assert(indexTemp is not null);
911+
887912
if (PrepareCopyToOptimization(spreadElement, rewrittenSpreadOperand) is not var (spanSliceMethod, spreadElementAsSpan, getLengthMethod, copyToMethod))
888913
return false;
889914

@@ -1177,13 +1202,21 @@ private BoundExpression CreateAndPopulateList(BoundCollectionExpression node, Ty
11771202
// Populate the span.
11781203
var spanGetItem = ((MethodSymbol)_factory.WellKnownMember(WellKnownMember.System_Span_T__get_Item)).AsMember((NamedTypeSymbol)spanTemp.Type);
11791204

1180-
// int index = 0;
1181-
BoundLocal indexTemp = _factory.StoreToTemp(
1182-
_factory.Literal(0),
1183-
out assignmentToTemp);
1184-
localsBuilder.Add(indexTemp);
1185-
sideEffects.Add(assignmentToTemp);
1205+
// indexTemp is null when we can use constant compile-time indices.
1206+
// indexTemp is non-null when we need a runtime-tracked index variable (for spread elements).
1207+
BoundLocal? indexTemp = null;
11861208

1209+
if (numberIncludingLastSpread != 0)
1210+
{
1211+
// int index = 0;
1212+
indexTemp = _factory.StoreToTemp(
1213+
_factory.Literal(0),
1214+
out assignmentToTemp);
1215+
localsBuilder.Add(indexTemp);
1216+
sideEffects.Add(assignmentToTemp);
1217+
}
1218+
1219+
int currentElementIndex = 0;
11871220
AddCollectionExpressionElements(
11881221
elements,
11891222
spanTemp,
@@ -1193,33 +1226,50 @@ private BoundExpression CreateAndPopulateList(BoundCollectionExpression node, Ty
11931226
addElement: (ArrayBuilder<BoundExpression> expressions, BoundExpression spanTemp, BoundExpression rewrittenValue, bool isLastElement) =>
11941227
{
11951228
Debug.Assert(spanTemp.Type is NamedTypeSymbol);
1196-
Debug.Assert(indexTemp.Type is { SpecialType: SpecialType.System_Int32 });
11971229

11981230
var expressionSyntax = rewrittenValue.Syntax;
11991231
var elementType = ((NamedTypeSymbol)spanTemp.Type).TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[0].Type;
12001232

1201-
// span[index] = element;
1202-
expressions.Add(
1203-
new BoundAssignmentOperator(
1204-
expressionSyntax,
1205-
_factory.Call(spanTemp, spanGetItem, indexTemp),
1206-
rewrittenValue,
1207-
isRef: false,
1208-
elementType));
1209-
if (!isLastElement)
1233+
if (indexTemp is null)
12101234
{
1211-
// index = index + 1;
1235+
// span[0] = element; span[1] = element; etc.
12121236
expressions.Add(
12131237
new BoundAssignmentOperator(
12141238
expressionSyntax,
1215-
indexTemp,
1216-
_factory.Binary(BinaryOperatorKind.Addition, indexTemp.Type, indexTemp, _factory.Literal(1)),
1239+
_factory.Call(spanTemp, spanGetItem, _factory.Literal(currentElementIndex)),
1240+
rewrittenValue,
12171241
isRef: false,
1218-
indexTemp.Type));
1242+
elementType));
1243+
currentElementIndex++;
1244+
}
1245+
else
1246+
{
1247+
// span[index] = element;
1248+
expressions.Add(
1249+
new BoundAssignmentOperator(
1250+
expressionSyntax,
1251+
_factory.Call(spanTemp, spanGetItem, indexTemp),
1252+
rewrittenValue,
1253+
isRef: false,
1254+
elementType));
1255+
if (!isLastElement)
1256+
{
1257+
// index = index + 1;
1258+
expressions.Add(
1259+
new BoundAssignmentOperator(
1260+
expressionSyntax,
1261+
indexTemp,
1262+
_factory.Binary(BinaryOperatorKind.Addition, indexTemp.Type, indexTemp, _factory.Literal(1)),
1263+
isRef: false,
1264+
indexTemp.Type));
1265+
}
12191266
}
12201267
},
12211268
tryOptimizeSpreadElement: (ArrayBuilder<BoundExpression> sideEffects, BoundExpression spanTemp, BoundCollectionExpressionSpreadElement spreadElement, BoundExpression rewrittenSpreadOperand) =>
12221269
{
1270+
// When we have spreads, we always need a runtime-tracked index variable.
1271+
Debug.Assert(indexTemp is not null);
1272+
12231273
if (PrepareCopyToOptimization(spreadElement, rewrittenSpreadOperand) is not var (spanSliceMethod, spreadElementAsSpan, getLengthMethod, copyToMethod))
12241274
return false;
12251275

0 commit comments

Comments
 (0)