Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collection expressions: emit Array.Empty<T>() for [] with target type T[] or IEnumerable<T> #69355

Merged
merged 4 commits into from
Aug 9, 2023
Merged

Collection expressions: emit Array.Empty<T>() for [] with target type T[] or IEnumerable<T> #69355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0d9c59d
Collection expressions: emit Array.Empty<T>() for [] with target type…
cston Aug 2, 2023
9444f58
Use Array.Empty<T>() for IReadOnlyCollection<T> and IReadOnlyList<T>
cston Aug 3, 2023
5d2e7a1
Test nested array of pointers
cston Aug 3, 2023
87cb4d6
Address feedback
cston Aug 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 22 additions & 20 deletions src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -675,29 +675,32 @@ private BoundCollectionExpression BindCollectionInitializerCollectionExpression(
}

var implicitReceiver = new BoundObjectOrCollectionValuePlaceholder(syntax, isNewInstance: true, targetType) { WasCompilerGenerated = true };
var collectionInitializerAddMethodBinder = this.WithAdditionalFlags(BinderFlags.CollectionInitializerAddMethod);
var builder = ArrayBuilder<BoundExpression>.GetInstance(node.Elements.Length);
foreach (var element in node.Elements)
if (node.Elements.Length > 0)
{
var result = element switch
var collectionInitializerAddMethodBinder = this.WithAdditionalFlags(BinderFlags.CollectionInitializerAddMethod);
foreach (var element in node.Elements)
{
BoundBadExpression => element,
BoundCollectionExpressionSpreadElement spreadElement => BindCollectionInitializerSpreadElementAddMethod(
(SpreadElementSyntax)spreadElement.Syntax,
spreadElement,
var result = element switch
{
BoundBadExpression => element,
BoundCollectionExpressionSpreadElement spreadElement => BindCollectionInitializerSpreadElementAddMethod(
(SpreadElementSyntax)spreadElement.Syntax,
spreadElement,
collectionInitializerAddMethodBinder,
implicitReceiver,
diagnostics),
_ => BindCollectionInitializerElementAddMethod(
(ExpressionSyntax)element.Syntax,
ImmutableArray.Create(element),
hasEnumerableInitializerType: true,
collectionInitializerAddMethodBinder,
implicitReceiver,
diagnostics),
_ => BindCollectionInitializerElementAddMethod(
(ExpressionSyntax)element.Syntax,
ImmutableArray.Create(element),
hasEnumerableInitializerType: true,
collectionInitializerAddMethodBinder,
diagnostics,
implicitReceiver),
};
result.WasCompilerGenerated = true;
builder.Add(result);
diagnostics,
implicitReceiver),
};
result.WasCompilerGenerated = true;
builder.Add(result);
}
}
return new BoundCollectionExpression(
syntax,
Expand All @@ -718,7 +721,6 @@ private BoundCollectionExpression BindListInterfaceCollectionExpression(
TypeSymbol elementType,
BindingDiagnosticBag diagnostics)
{
// https://github.com/dotnet/roslyn/issues/68785: Emit [] as Array.Empty<T>() rather than a List<T>.
var result = BindCollectionInitializerCollectionExpression(
node,
CollectionExpressionTypeKind.ListInterface,
Expand Down
79 changes: 57 additions & 22 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1469,38 +1469,73 @@ private BoundExpression BuildParamsArray(
// if it's available. However, we also disable the optimization if we're in an expression lambda, the
// point of which is just to represent the semantics of an operation, and we don't know that all consumers
// of expression lambdas will appropriately understand Array.Empty<T>().
// We disable it for pointer types as well, since they cannot be used as Type Arguments.
if (arrayArgs.Length == 0
&& !_inExpressionLambda
&& paramArrayType is ArrayTypeSymbol ats // could be false if there's a semantic error, e.g. the params parameter type isn't an array
&& !ats.ElementType.IsPointerOrFunctionPointer())
&& paramArrayType is ArrayTypeSymbol ats) // could be false if there's a semantic error, e.g. the params parameter type isn't an array
{
MethodSymbol? arrayEmpty = _compilation.GetWellKnownTypeMember(WellKnownMember.System_Array__Empty) as MethodSymbol;
if (arrayEmpty != null) // will be null if Array.Empty<T> doesn't exist in reference assemblies
BoundExpression? arrayEmpty = CreateArrayEmptyCallIfAvailable(syntax, ats.ElementType);
if (arrayEmpty is { })
{
_diagnostics.ReportUseSite(arrayEmpty, syntax);
// return an invocation of "Array.Empty<T>()"
arrayEmpty = arrayEmpty.Construct(ImmutableArray.Create(ats.ElementType));
return new BoundCall(
syntax,
null,
arrayEmpty,
ImmutableArray<BoundExpression>.Empty,
default(ImmutableArray<string>),
default(ImmutableArray<RefKind>),
isDelegateCall: false,
expanded: false,
invokedAsExtensionMethod: false,
argsToParamsOpt: default(ImmutableArray<int>),
defaultArguments: default(BitVector),
resultKind: LookupResultKind.Viable,
type: arrayEmpty.ReturnType);
return arrayEmpty;
}
}

return CreateParamArrayArgument(syntax, paramArrayType, arrayArgs, _compilation, this);
}

private BoundExpression CreateEmptyArray(SyntaxNode syntax, ArrayTypeSymbol arrayType)
{
BoundExpression? arrayEmpty = CreateArrayEmptyCallIfAvailable(syntax, arrayType.ElementType);
if (arrayEmpty is { })
{
return arrayEmpty;
}
// new T[0]
return new BoundArrayCreation(
Copy link
Member

@jcouv jcouv Aug 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: consider adding equivalent pseudo-code // new ArrayType[0] #Closed

syntax,
ImmutableArray.Create<BoundExpression>(
new BoundLiteral(
syntax,
ConstantValue.Create(0),
_compilation.GetSpecialType(SpecialType.System_Int32))),
initializerOpt: null,
arrayType)
{ WasCompilerGenerated = true };
}

private BoundExpression? CreateArrayEmptyCallIfAvailable(SyntaxNode syntax, TypeSymbol elementType)
{
if (elementType.IsPointerOrFunctionPointer())
{
// Pointer types cannot be used as type arguments.
return null;
}

MethodSymbol? arrayEmpty = _compilation.GetWellKnownTypeMember(WellKnownMember.System_Array__Empty) as MethodSymbol;
if (arrayEmpty is null) // will be null if Array.Empty<T> doesn't exist in reference assemblies
{
return null;
}

_diagnostics.ReportUseSite(arrayEmpty, syntax);
// return an invocation of "Array.Empty<T>()"
arrayEmpty = arrayEmpty.Construct(ImmutableArray.Create(elementType));
return new BoundCall(
syntax,
null,
arrayEmpty,
ImmutableArray<BoundExpression>.Empty,
default(ImmutableArray<string>),
default(ImmutableArray<RefKind>),
isDelegateCall: false,
expanded: false,
invokedAsExtensionMethod: false,
argsToParamsOpt: default(ImmutableArray<int>),
defaultArguments: default(BitVector),
resultKind: LookupResultKind.Viable,
type: arrayEmpty.ReturnType);
}

private static BoundExpression CreateParamArrayArgument(SyntaxNode syntax,
TypeSymbol paramArrayType,
ImmutableArray<BoundExpression> arrayArgs,
Expand Down
64 changes: 44 additions & 20 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CollectionExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,23 +81,27 @@ private BoundExpression VisitArrayOrSpanCollectionExpression(BoundCollectionExpr
else
{
int arrayLength = elements.Length;
// https://github.com/dotnet/roslyn/issues/68785: Emit [] as Array.Empty<T>() rather than a List<T>.
var initialization = (arrayLength == 0)
? null
: new BoundArrayInitialization(
syntax,
isInferred: false,
elements.SelectAsArray(e => VisitExpression(e)));
array = new BoundArrayCreation(
syntax,
ImmutableArray.Create<BoundExpression>(
new BoundLiteral(
if (arrayLength == 0)
{
array = CreateEmptyArray(syntax, arrayType);
}
else
{
var initialization = new BoundArrayInitialization(
syntax,
ConstantValue.Create(arrayLength),
_compilation.GetSpecialType(SpecialType.System_Int32))),
initialization,
arrayType)
{ WasCompilerGenerated = true };
isInferred: false,
elements.SelectAsArray(e => VisitExpression(e)));
array = new BoundArrayCreation(
syntax,
ImmutableArray.Create<BoundExpression>(
new BoundLiteral(
syntax,
ConstantValue.Create(arrayLength),
_compilation.GetSpecialType(SpecialType.System_Int32))),
initialization,
arrayType)
{ WasCompilerGenerated = true };
}
}

if (spanConstructor is null)
Expand Down Expand Up @@ -156,11 +160,31 @@ private BoundExpression VisitCollectionInitializerCollectionExpression(BoundColl
private BoundExpression VisitListInterfaceCollectionExpression(BoundCollectionExpression node)
{
Debug.Assert(!_inExpressionLambda);
Debug.Assert(node.Type is { });
Debug.Assert(node.Type is NamedTypeSymbol);

var collectionType = (NamedTypeSymbol)node.Type;
BoundExpression arrayOrList;

// Use Array.Empty<T>() rather than List<T> for an empty collection expression when
Copy link
Contributor

@RikkiGibson RikkiGibson Aug 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know things are in flux here but the implementation seems fine.

// the target type is IEnumerable<T>, IReadOnlyCollection<T>, or IReadOnlyList<T>.
if (node.Elements.Length == 0 &&
collectionType is
{
OriginalDefinition.SpecialType:
SpecialType.System_Collections_Generic_IEnumerable_T or
SpecialType.System_Collections_Generic_IReadOnlyCollection_T or
SpecialType.System_Collections_Generic_IReadOnlyList_T,
TypeArgumentsWithAnnotationsNoUseSiteDiagnostics: [var elementType]
})
{
arrayOrList = CreateEmptyArray(node.Syntax, ArrayTypeSymbol.CreateSZArray(_compilation.Assembly, elementType));
}
else
{
arrayOrList = VisitCollectionInitializerCollectionExpression(node, collectionType);
}

// https://github.com/dotnet/roslyn/issues/68785: Emit [] as Array.Empty<T>() rather than a List<T>.
var list = VisitCollectionInitializerCollectionExpression(node, node.Type);
return _factory.Convert(node.Type, list);
return _factory.Convert(collectionType, arrayOrList);
}

private BoundExpression VisitCollectionBuilderCollectionExpression(BoundCollectionExpression node)
Expand Down
Loading