Skip to content

Commit

Permalink
use test case parameterized values instead of hardcoded ones (#256)
Browse files Browse the repository at this point in the history
Test was using [2, 1] hardcoded collection expression instead of the parameterized values whence collection
expressions larger than 2 were not being tested.

Since this length is used to determine how the initialization is performed optimized initialization (length >= 3)
was not being tested.

Fixing this revealed a bug in the logic that selects optimized/unoptimized initialization. AFAICS optimization is only
applicable when the target of the collection expression (i.e, a variable assignment/initialization or argument passing)
is a container (array/collection/etc) of primitive types but the code was not taking this into consideration.
  • Loading branch information
adrianoc committed Sep 29, 2024
1 parent 481d82e commit f337d8c
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ public void ImplicitTypeConversions_Are_Applied([Values("List<Foo>", "Foo[]", "S
using System.Collections.Generic;
using System;
{{targetType}} items = [1, 2];
{{targetType}} items = {{items}};
// We can´t rely on a foreach (to simplify the code) due to issue #306
for(var i = {{lengthExtractor}} - 1 ; i >= 0; i--) System.Console.Write(items[i].Value);
for(var i = 0; i < {{lengthExtractor}}; i++) System.Console.Write(items[i].Value);
struct Foo
{
Expand All @@ -91,6 +91,9 @@ struct Foo
public int Value;
}
""",
Regex.Replace(items, @"[\[\],\s+]", ""),
expectedILError);
}
"21",
expectedILError);
}
Expand Down
2 changes: 1 addition & 1 deletion Cecilifier.Core/AST/CollectionExpressionProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ private static void HandleAssignmentToArray(ExpressionVisitor visitor, Collectio
visitor.Context.EmitCilInstruction(visitor.ILVariable, OpCodes.Ldc_I4, node.Elements.Count);
visitor.Context.EmitCilInstruction(visitor.ILVariable, OpCodes.Newarr, visitor.Context.TypeResolver.Resolve(arrayTypeSymbol.ElementType));

if (PrivateImplementationDetailsGenerator.IsApplicableTo(node))
if (PrivateImplementationDetailsGenerator.IsApplicableTo(node, visitor.Context))
ArrayInitializationProcessor.InitializeOptimized(visitor, arrayTypeSymbol.ElementType, node.Elements);
else
ArrayInitializationProcessor.InitializeUnoptimized<CollectionElementSyntax>(visitor, arrayTypeSymbol.ElementType, node.Elements, visitor.Context.SemanticModel.GetOperation(node));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ private static DefinitionVariable GetOrCreatePrivateImplementationDetailsTypeVar
/// Encodes rules used by C# compiler to decide whether to apply the array/stackalloc initialization optimization.
///
/// Note that as of version 4.6.1 of Roslyn, the empirically discovered rules are:
/// 1. Any array initialization expression with length > 2 are optimized
/// 1. Array initialization expression with length > 2 are optimized if the element type is a primitive.
/// 2. Stackalloc initialization is only considered if the type being allocated is byte/sbyte/bool plus same length rule above
/// </summary>
/// <param name="node"></param>
Expand Down Expand Up @@ -405,7 +405,14 @@ static bool HasCompatibleType(InitializerExpressionSyntax expression, IVisitorCo
}
}

public static bool IsApplicableTo(CollectionExpressionSyntax node) => IsLargeEnoughToWarrantOptimization(node.Elements);

public static bool IsApplicableTo(CollectionExpressionSyntax node, IVisitorContext context)
{
if (!IsLargeEnoughToWarrantOptimization(node.Elements))
return false;

var operation = context.SemanticModel.GetOperation(node);
return operation?.Type.ElementTypeSymbolOf().IsPrimitiveType() == true;
}

static bool IsLargeEnoughToWarrantOptimization<TElement>(SeparatedSyntaxList<TElement> elements) where TElement : SyntaxNode => elements.Count > 2;
}

0 comments on commit f337d8c

Please sign in to comment.