Skip to content

Commit b5ed80e

Browse files
Improve documentation and inline condition check
Improved the documentation to clarify that indexTemp's null state indicates whether constant compile-time indices or runtime-tracked index is used. Changed the condition from `numberIncludingLastSpread > 0` to `numberIncludingLastSpread != 0` for consistency. Removed unnecessary Debug.Assert calls that are redundant with the null check. Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>
1 parent f9b7579 commit b5ed80e

File tree

1 file changed

+8
-12
lines changed

1 file changed

+8
-12
lines changed

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

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

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

834-
// When there are no spread elements, we can use constant indices for better codegen.
835-
// When there are spread elements, we need a mutable index variable for the CopyTo optimization.
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).
836836
BoundLocal? indexTemp = null;
837837

838-
if (numberIncludingLastSpread > 0)
838+
if (numberIncludingLastSpread != 0)
839839
{
840840
// int index = 0;
841841
indexTemp = _factory.StoreToTemp(
@@ -883,8 +883,6 @@ private BoundExpression CreateAndPopulateArray(BoundCollectionExpression node, A
883883
}
884884
else
885885
{
886-
Debug.Assert(indexTemp.Type is { SpecialType: SpecialType.System_Int32 });
887-
888886
// array[index] = element;
889887
expressions.Add(
890888
new BoundAssignmentOperator(
@@ -908,7 +906,7 @@ private BoundExpression CreateAndPopulateArray(BoundCollectionExpression node, A
908906
},
909907
tryOptimizeSpreadElement: (ArrayBuilder<BoundExpression> sideEffects, BoundExpression arrayTemp, BoundCollectionExpressionSpreadElement spreadElement, BoundExpression rewrittenSpreadOperand) =>
910908
{
911-
Debug.Assert(indexTemp is not null, "Should not have spread elements when using constant indices");
909+
Debug.Assert(indexTemp is not null);
912910

913911
if (PrepareCopyToOptimization(spreadElement, rewrittenSpreadOperand) is not var (spanSliceMethod, spreadElementAsSpan, getLengthMethod, copyToMethod))
914912
return false;
@@ -1203,11 +1201,11 @@ private BoundExpression CreateAndPopulateList(BoundCollectionExpression node, Ty
12031201
// Populate the span.
12041202
var spanGetItem = ((MethodSymbol)_factory.WellKnownMember(WellKnownMember.System_Span_T__get_Item)).AsMember((NamedTypeSymbol)spanTemp.Type);
12051203

1206-
// When there are no spread elements, we can use constant indices for better codegen.
1207-
// When there are spread elements, we need a mutable index variable for the CopyTo optimization.
1204+
// indexTemp is null when we can use constant compile-time indices.
1205+
// indexTemp is non-null when we need a runtime-tracked index variable (for spread elements).
12081206
BoundLocal? indexTemp = null;
12091207

1210-
if (numberIncludingLastSpread > 0)
1208+
if (numberIncludingLastSpread != 0)
12111209
{
12121210
// int index = 0;
12131211
indexTemp = _factory.StoreToTemp(
@@ -1245,8 +1243,6 @@ private BoundExpression CreateAndPopulateList(BoundCollectionExpression node, Ty
12451243
}
12461244
else
12471245
{
1248-
Debug.Assert(indexTemp.Type is { SpecialType: SpecialType.System_Int32 });
1249-
12501246
// span[index] = element;
12511247
expressions.Add(
12521248
new BoundAssignmentOperator(
@@ -1270,7 +1266,7 @@ private BoundExpression CreateAndPopulateList(BoundCollectionExpression node, Ty
12701266
},
12711267
tryOptimizeSpreadElement: (ArrayBuilder<BoundExpression> sideEffects, BoundExpression spanTemp, BoundCollectionExpressionSpreadElement spreadElement, BoundExpression rewrittenSpreadOperand) =>
12721268
{
1273-
Debug.Assert(indexTemp is not null, "Should not have spread elements when using constant indices");
1269+
Debug.Assert(indexTemp is not null);
12741270

12751271
if (PrepareCopyToOptimization(spreadElement, rewrittenSpreadOperand) is not var (spanSliceMethod, spreadElementAsSpan, getLengthMethod, copyToMethod))
12761272
return false;

0 commit comments

Comments
 (0)