Skip to content

Commit

Permalink
Collection expressions: use inline arrays for collection expressions …
Browse files Browse the repository at this point in the history
…with span target types
  • Loading branch information
cston committed Aug 5, 2023
1 parent aa16063 commit e72459d
Show file tree
Hide file tree
Showing 21 changed files with 1,897 additions and 334 deletions.
16 changes: 15 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3960,7 +3960,9 @@ internal uint GetValEscape(BoundExpression expr, uint scopeOfTheContainingExpres
return GetValEscape(switchExpr.SwitchArms.SelectAsArray(a => a.Value), scopeOfTheContainingExpression);

case BoundKind.CollectionExpression:
return CallingMethodScope;
return HasLocalScope((BoundCollectionExpression)expr)
? scopeOfTheContainingExpression
: CallingMethodScope;

default:
// in error situations some unexpected nodes could make here
Expand All @@ -3971,6 +3973,13 @@ internal uint GetValEscape(BoundExpression expr, uint scopeOfTheContainingExpres
}
}

private static bool HasLocalScope(BoundCollectionExpression expr)
{
// A non-empty collection expression with span type may be stored on
// the stack, so the expression is assumed to have local scope.
return expr.Type?.IsRefLikeType == true && expr.Elements.Length > 0;
}

private uint GetTupleValEscape(ImmutableArray<BoundExpression> elements, uint scopeOfTheContainingExpression)
{
uint narrowestScope = scopeOfTheContainingExpression;
Expand Down Expand Up @@ -4489,6 +4498,11 @@ internal bool CheckValEscape(SyntaxNode node, BoundExpression expr, uint escapeF
return true;

case BoundKind.CollectionExpression:
if (HasLocalScope((BoundCollectionExpression)expr) && escapeTo < _localScopeDepth)
{
Error(diagnostics, ErrorCode.ERR_CollectionExpressionEscape, node, expr.Type);
return false;
}
return true;

default:
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -6849,6 +6849,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_CollectionBuilderAttributeInvalidMethodName" xml:space="preserve">
<value>The CollectionBuilderAttribute method name is invalid.</value>
</data>
<data name="ERR_CollectionExpressionEscape" xml:space="preserve">
<value>A collection expression of type '{0}' cannot be used in this context because it may be exposed outside of the current scope.</value>
</data>
<data name="ERR_EqualityContractRequiresGetter" xml:space="preserve">
<value>Record equality contract property '{0}' must have a get accessor.</value>
</data>
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2266,6 +2266,7 @@ internal enum ErrorCode
ERR_OutAttrOnRefReadonlyParam = 9199,
WRN_RefReadonlyParameterDefaultValue = 9200,
WRN_UseDefViolationRefField = 9201,
ERR_CollectionExpressionEscape = 9202,

#endregion

Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2398,6 +2398,7 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.WRN_RefReadonlyParameterDefaultValue:
case ErrorCode.WRN_ByValArraySizeConstRequired:
case ErrorCode.WRN_UseDefViolationRefField:
case ErrorCode.ERR_CollectionExpressionEscape:
return false;
default:
// NOTE: All error codes must be explicitly handled in this switch statement
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ internal sealed partial class LocalRewriter : BoundTreeRewriterWithStackGuard
private readonly int _topLevelMethodOrdinal;
private DelegateCacheRewriter? _lazyDelegateCacheRewriter;
private bool _inExpressionLambda;
private ArrayBuilder<LocalSymbol>? _additionalLocals;

/// <summary>
/// The original body of the current lambda or local function body, or null if not currently lowering a lambda.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Diagnostics;
using System.Collections.Immutable;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.PooledObjects;
Expand All @@ -20,26 +19,37 @@ public override BoundNode VisitBlock(BoundBlock node)
}

var builder = ArrayBuilder<BoundStatement>.GetInstance();
VisitStatementSubList(builder, node.Statements);
var previousLocals = _additionalLocals;
_additionalLocals = ArrayBuilder<LocalSymbol>.GetInstance();

var additionalLocals = TemporaryArray<LocalSymbol>.Empty;

BoundBlockInstrumentation? instrumentation = null;
if (Instrument)
try
{
Instrumenter.InstrumentBlock(node, this, ref additionalLocals, out var prologue, out var epilogue, out instrumentation);
if (prologue != null)
{
builder.Insert(0, prologue);
}
VisitStatementSubList(builder, node.Statements);

var additionalLocals = TemporaryArray<LocalSymbol>.Empty; // PROTOTYPE: Use _additionalLocals rather than creating a TemporaryArray<LocalSymbol>?

if (epilogue != null)
BoundBlockInstrumentation? instrumentation = null;
if (Instrument)
{
builder.Add(epilogue);
Instrumenter.InstrumentBlock(node, this, ref additionalLocals, out var prologue, out var epilogue, out instrumentation);
if (prologue != null)
{
builder.Insert(0, prologue);
}

if (epilogue != null)
{
builder.Add(epilogue);
}
}
}

return new BoundBlock(node.Syntax, node.Locals.AddRange(additionalLocals), node.LocalFunctions, node.HasUnsafeModifier, instrumentation, builder.ToImmutableAndFree(), node.HasErrors);
return new BoundBlock(node.Syntax, node.Locals.AddRange(_additionalLocals).AddRange(additionalLocals), node.LocalFunctions, node.HasUnsafeModifier, instrumentation, builder.ToImmutableAndFree(), node.HasErrors);
}
finally
{
_additionalLocals.Free();
_additionalLocals = previousLocals;
}
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Linq;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Collections;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp
Expand All @@ -31,7 +32,7 @@ internal sealed partial class LocalRewriter
case CollectionExpressionTypeKind.Span:
case CollectionExpressionTypeKind.ReadOnlySpan:
Debug.Assert(elementType is { });
return VisitArrayOrSpanCollectionExpression(node, node.Type, TypeWithAnnotations.Create(elementType));
return VisitArrayOrSpanCollectionExpression(node, collectionTypeKind, node.Type, TypeWithAnnotations.Create(elementType));
case CollectionExpressionTypeKind.CollectionBuilder:
return VisitCollectionBuilderCollectionExpression(node);
case CollectionExpressionTypeKind.ListInterface:
Expand All @@ -46,26 +47,44 @@ internal sealed partial class LocalRewriter
}
}

private BoundExpression VisitArrayOrSpanCollectionExpression(BoundCollectionExpression node, TypeSymbol collectionType, TypeWithAnnotations elementType)
private BoundExpression VisitArrayOrSpanCollectionExpression(BoundCollectionExpression node, CollectionExpressionTypeKind collectionTypeKind, TypeSymbol collectionType, TypeWithAnnotations elementType)
{
Debug.Assert(!_inExpressionLambda);
//Debug.Assert(_additionalLocals is { }); // PROTOTYPE: Support inline array with static and instance field initializers: static readonly MyCollection<int> _instance = [1, 2, 3];

var syntax = node.Syntax;
var elements = node.Elements;
MethodSymbol? spanConstructor = null;

// PROTOTYPE: Assert the inline array temp is associated with the expected local scope.

var arrayType = collectionType as ArrayTypeSymbol;
if (arrayType is null)
{
Debug.Assert(collectionType.Name is "Span" or "ReadOnlySpan");
// We're constructing a Span<T> or ReadOnlySpan<T> rather than T[].
var spanType = (NamedTypeSymbol)collectionType;

Debug.Assert(collectionTypeKind is CollectionExpressionTypeKind.Span or CollectionExpressionTypeKind.ReadOnlySpan);
Debug.Assert(spanType.OriginalDefinition.Equals(_compilation.GetWellKnownType(
collectionTypeKind == CollectionExpressionTypeKind.Span ? WellKnownType.System_Span_T : WellKnownType.System_ReadOnlySpan_T), TypeCompareKind.AllIgnoreOptions));
Debug.Assert(elementType.Equals(spanType.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[0], TypeCompareKind.AllIgnoreOptions));

if (ShouldUseInlineArray(node) &&
_additionalLocals is { }) // PROTOTYPE: Support inline array with static and instance field initializers: static readonly MyCollection<int> _instance = [1, 2, 3];
{
return CreateAndPopulateSpanFromInlineArray(
syntax,
elementType,
elements,
asReadOnlySpan: collectionTypeKind == CollectionExpressionTypeKind.ReadOnlySpan,
_additionalLocals);
}

arrayType = ArrayTypeSymbol.CreateSZArray(_compilation.Assembly, elementType);
spanConstructor = ((MethodSymbol)_compilation.GetWellKnownTypeMember(
collectionType.Name == "Span" ? WellKnownMember.System_Span_T__ctor_Array : WellKnownMember.System_ReadOnlySpan_T__ctor_Array)!).AsMember(spanType);
collectionTypeKind == CollectionExpressionTypeKind.Span ? WellKnownMember.System_Span_T__ctor_Array : WellKnownMember.System_ReadOnlySpan_T__ctor_Array)!).AsMember(spanType);
}

var elements = node.Elements;
BoundExpression array;

if (elements.Any(i => i is BoundCollectionExpressionSpreadElement))
Expand Down Expand Up @@ -168,8 +187,6 @@ private BoundExpression VisitCollectionBuilderCollectionExpression(BoundCollecti
Debug.Assert(!_inExpressionLambda);
Debug.Assert(node.Type is { });

var syntax = node.Syntax;
var elements = node.Elements;
var constructMethod = node.CollectionBuilderMethod;

Debug.Assert(constructMethod is { });
Expand All @@ -179,24 +196,10 @@ private BoundExpression VisitCollectionBuilderCollectionExpression(BoundCollecti
Debug.Assert(spanType.OriginalDefinition.Equals(_compilation.GetWellKnownType(WellKnownType.System_ReadOnlySpan_T), TypeCompareKind.AllIgnoreOptions));

var elementType = spanType.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[0];
var locals = ArrayBuilder<LocalSymbol>.GetInstance();
var sideEffects = ArrayBuilder<BoundExpression>.GetInstance();
BoundExpression span;

if (elements.Length > 0
&& !elements.Any(i => i is BoundCollectionExpressionSpreadElement)
&& _compilation.Assembly.RuntimeSupportsInlineArrayTypes
&& (!constructMethod.ReturnType.IsRefLikeType || constructMethod.Parameters[0].EffectiveScope == ScopedKind.ScopedValue))
{
span = CreateAndPopulateInlineArray(syntax, elementType, elements, locals, sideEffects);
}
else
{
span = VisitArrayOrSpanCollectionExpression(node, spanType, elementType);
}
BoundExpression span = VisitArrayOrSpanCollectionExpression(node, CollectionExpressionTypeKind.ReadOnlySpan, spanType, elementType);

var call = new BoundCall(
syntax,
return new BoundCall(
node.Syntax,
receiverOpt: null,
method: constructMethod,
arguments: ImmutableArray.Create(span),
Expand All @@ -209,26 +212,30 @@ private BoundExpression VisitCollectionBuilderCollectionExpression(BoundCollecti
defaultArguments: default,
resultKind: LookupResultKind.Viable,
type: constructMethod.ReturnType);
}

return new BoundSequence(
syntax,
locals.ToImmutableAndFree(),
sideEffects.ToImmutableAndFree(),
call,
call.Type);
private bool ShouldUseInlineArray(BoundCollectionExpression node)
{
var elements = node.Elements;
return elements.Length > 0 &&
!elements.Any(i => i is BoundCollectionExpressionSpreadElement) &&
_compilation.Assembly.RuntimeSupportsInlineArrayTypes;
}

private BoundExpression CreateAndPopulateInlineArray(
private BoundExpression CreateAndPopulateSpanFromInlineArray(
SyntaxNode syntax,
TypeWithAnnotations elementType,
ImmutableArray<BoundExpression> elements,
ArrayBuilder<LocalSymbol> locals,
ArrayBuilder<BoundExpression> sideEffects)
bool asReadOnlySpan,
ArrayBuilder<LocalSymbol> locals)
{
Debug.Assert(elements.Length > 0);
Debug.Assert(_factory.ModuleBuilderOpt is { });
Debug.Assert(_diagnostics.DiagnosticBag is { });
Debug.Assert(_compilation.Assembly.RuntimeSupportsInlineArrayTypes);
#if DEBUG
int nLocalsBefore = locals.Count;
#endif

int arrayLength = elements.Length;
var inlineArrayType = _factory.ModuleBuilderOpt.EnsureInlineArrayTypeExists(syntax, _factory, arrayLength, _diagnostics.DiagnosticBag).Construct(ImmutableArray.Create(elementType));
Expand All @@ -242,6 +249,7 @@ private BoundExpression CreateAndPopulateInlineArray(
// var tmp = new <>y__InlineArrayN<ElementType>();
BoundAssignmentOperator assignmentToTemp;
BoundLocal inlineArrayLocal = _factory.StoreToTemp(new BoundDefaultExpression(syntax, inlineArrayType), out assignmentToTemp, isKnownToReferToTempIfReferenceType: true);
var sideEffects = ArrayBuilder<BoundExpression>.GetInstance();
sideEffects.Add(assignmentToTemp);
locals.Add(inlineArrayLocal.LocalSymbol);

Expand All @@ -259,14 +267,29 @@ private BoundExpression CreateAndPopulateInlineArray(

// Get a span to the inline array.
// ... InlineArrayAsReadOnlySpan<<>y__InlineArrayN<ElementType>, ElementType>(in tmp, N)
var inlineArrayAsReadOnlySpan = _factory.ModuleBuilderOpt.EnsureInlineArrayAsReadOnlySpanExists(syntax, _factory.WellKnownType(WellKnownType.System_ReadOnlySpan_T), intType, _diagnostics.DiagnosticBag).
Construct(ImmutableArray.Create(TypeWithAnnotations.Create(inlineArrayType), elementType));
return _factory.Call(
// or
// ... InlineArrayAsSpan<<>y__InlineArrayN<ElementType>, ElementType>(ref tmp, N)
MethodSymbol inlineArrayAsSpan = asReadOnlySpan ?
_factory.ModuleBuilderOpt.EnsureInlineArrayAsReadOnlySpanExists(syntax, _factory.WellKnownType(WellKnownType.System_ReadOnlySpan_T), intType, _diagnostics.DiagnosticBag) :
_factory.ModuleBuilderOpt.EnsureInlineArrayAsSpanExists(syntax, _factory.WellKnownType(WellKnownType.System_Span_T), intType, _diagnostics.DiagnosticBag);
inlineArrayAsSpan = inlineArrayAsSpan.Construct(ImmutableArray.Create(TypeWithAnnotations.Create(inlineArrayType), elementType));
var span = _factory.Call(
receiver: null,
inlineArrayAsReadOnlySpan,
inlineArrayAsSpan,
inlineArrayLocal,
_factory.Literal(arrayLength),
useStrictArgumentRefKinds: true);

Debug.Assert(span.Type is { });
#if DEBUG
Debug.Assert(locals.Count == nLocalsBefore + 1); // Should be exactly one local, for the inline array.
#endif
return new BoundSequence(
syntax,
locals: ImmutableArray<LocalSymbol>.Empty,
sideEffects.ToImmutableAndFree(),
span,
span.Type);
}

private BoundExpression MakeCollectionExpressionSpreadElement(BoundCollectionExpressionSpreadElement initializer)
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit e72459d

Please sign in to comment.