Skip to content

Commit

Permalink
Use BoundCollectionInitializer for spread iterator body
Browse files Browse the repository at this point in the history
  • Loading branch information
@cston
cston committed Nov 18, 2023
1 parent 5f2379c commit 5ae2a11
Show file tree
Hide file tree
Showing 5 changed files with 428 additions and 272 deletions.
13 changes: 7 additions & 6 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5968,12 +5968,13 @@ private BoundCollectionExpressionSpreadElement BindCollectionExpressionSpreadEle

Debug.Assert(enumeratorInfo.ElementType is { }); // ElementType is set always, even for IEnumerable.
var addElementPlaceholder = new BoundValuePlaceholder(syntax, enumeratorInfo.ElementType);
var addMethodInvocation = collectionInitializerAddMethodBinder.MakeInvocationExpression(
syntax,
implicitReceiver,
methodName: WellKnownMemberNames.CollectionInitializerAddMethodName,
args: ImmutableArray.Create<BoundExpression>(addElementPlaceholder),
diagnostics);
var addMethodInvocation = BindCollectionInitializerElementAddMethod(
syntax.Expression,
ImmutableArray.Create((BoundExpression)addElementPlaceholder),
hasEnumerableInitializerType: true,
collectionInitializerAddMethodBinder,
diagnostics,
implicitReceiver);
return element.Update(
element.Expression,
expressionPlaceholder: element.ExpressionPlaceholder,
Expand Down
7 changes: 4 additions & 3 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3496,9 +3496,10 @@ protected override void VisitStatement(BoundStatement statement)
switch (element)
{
case BoundCollectionElementInitializer initializer:
var collectionType = initializer.AddMethod.ContainingType;

var completion = VisitCollectionElementInitializer(initializer, collectionType,
Debug.Assert(node.Placeholder is { });
var completion = VisitCollectionElementInitializer(
initializer,
containingType: node.Placeholder.Type,
delayCompletionForType: false /* All collection expressions are target-typed */);

Debug.Assert(completion is null);
Expand Down
41 changes: 27 additions & 14 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CollectionExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,17 +181,20 @@ private BoundExpression VisitCollectionInitializerCollectionExpression(BoundColl

foreach (var element in elements)
{
var rewrittenElement = element switch
{
BoundCollectionElementInitializer collectionInitializer => MakeCollectionInitializer(temp, collectionInitializer),
BoundDynamicCollectionElementInitializer dynamicInitializer => MakeDynamicCollectionInitializer(temp, dynamicInitializer),
BoundCollectionExpressionSpreadElement spreadElement =>
MakeCollectionExpressionSpreadElement(
spreadElement,
VisitExpression(spreadElement.Expression),
static (rewriter, iteratorBody) => rewriter.VisitStatement(iteratorBody)!),
_ => throw ExceptionUtilities.UnexpectedValue(element)
};
var rewrittenElement = element is BoundCollectionExpressionSpreadElement spreadElement ?
MakeCollectionExpressionSpreadElement(
spreadElement,
VisitExpression(spreadElement.Expression),
iteratorBody =>
{
var syntax = iteratorBody.Syntax;
var rewrittenValue = rewriteCollectionInitializer(temp, ((BoundExpressionStatement)iteratorBody).Expression);
// MakeCollectionInitializer() may return null if Add() is marked [Conditional].
return rewrittenValue is { } ?
new BoundExpressionStatement(syntax, rewrittenValue) :
new BoundNoOpStatement(syntax, NoOpStatementFlavor.Default);
}) :
rewriteCollectionInitializer(temp, (BoundExpression)element);
if (rewrittenElement != null)
{
sideEffects.Add(rewrittenElement);
Expand All @@ -206,6 +209,16 @@ private BoundExpression VisitCollectionInitializerCollectionExpression(BoundColl
sideEffects.ToImmutableAndFree(),
temp,
collectionType);

BoundExpression? rewriteCollectionInitializer(BoundLocal rewrittenReceiver, BoundExpression expressionElement)
{
return expressionElement switch
{
BoundCollectionElementInitializer collectionInitializer => MakeCollectionInitializer(rewrittenReceiver, collectionInitializer),
BoundDynamicCollectionElementInitializer dynamicInitializer => MakeDynamicCollectionInitializer(rewrittenReceiver, dynamicInitializer),
var e => throw ExceptionUtilities.UnexpectedValue(e)
};
}
}

private BoundExpression VisitListInterfaceCollectionExpression(BoundCollectionExpression node)
Expand Down Expand Up @@ -722,7 +735,7 @@ private void AddCollectionExpressionElements(
var rewrittenElement = MakeCollectionExpressionSpreadElement(
spreadElement,
rewrittenExpression,
(_, iteratorBody) =>
iteratorBody =>
{
var rewrittenValue = VisitExpression(((BoundExpressionStatement)iteratorBody).Expression);
var builder = ArrayBuilder<BoundExpression>.GetInstance();
Expand Down Expand Up @@ -796,7 +809,7 @@ BoundExpression add(BoundExpression? sum, BoundExpression value)
private BoundExpression MakeCollectionExpressionSpreadElement(
BoundCollectionExpressionSpreadElement node,
BoundExpression rewrittenExpression,
Func<LocalRewriter, BoundStatement, BoundStatement> getRewrittenBody)
Func<BoundStatement, BoundStatement> rewriteBody)
{
var enumeratorInfo = node.EnumeratorInfoOpt;
var convertedExpression = (BoundConversion?)node.Conversion;
Expand All @@ -816,7 +829,7 @@ private BoundExpression MakeCollectionExpressionSpreadElement(
var iterationLocal = _factory.Local(iterationVariable);

AddPlaceholderReplacement(elementPlaceholder, iterationLocal);
var rewrittenBody = getRewrittenBody(this, iteratorBody);
var rewrittenBody = rewriteBody(iteratorBody);
RemovePlaceholderReplacement(elementPlaceholder);

var iterationVariables = ImmutableArray.Create(iterationVariable);
Expand Down
44 changes: 12 additions & 32 deletions src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1225,7 +1225,7 @@ private IOperation CreateBoundCollectionExpression(BoundCollectionExpression exp
ITypeSymbol? collectionType = expr.GetPublicTypeSymbol();
bool isImplicit = expr.WasCompilerGenerated;
IMethodSymbol? constructMethod = getConstructMethod((CSharpCompilation)_semanticModel.Compilation, expr).GetPublicSymbol();
ImmutableArray<IOperation> elements = expr.Elements.SelectAsArray(e => CreateBoundCollectionExpressionElement(expr, e));
ImmutableArray<IOperation> elements = expr.Elements.SelectAsArray(e => CreateBoundCollectionExpressionElement(e));
return new CollectionExpressionOperation(
constructMethod,
elements,
Expand All @@ -1250,7 +1250,7 @@ private IOperation CreateBoundCollectionExpression(BoundCollectionExpression exp
case CollectionExpressionTypeKind.CollectionBuilder:
return expr.CollectionBuilderMethod;
case CollectionExpressionTypeKind.ImmutableArray:
// PROTOTYPE: Return the [CollectionBuilder] method?
// https://github.com/dotnet/roslyn/issues/70880: Return the [CollectionBuilder] method.
return null;
case CollectionExpressionTypeKind.List:
Debug.Assert(expr.Type is { });
Expand All @@ -1261,42 +1261,22 @@ private IOperation CreateBoundCollectionExpression(BoundCollectionExpression exp
}
}

private IOperation CreateBoundCollectionExpressionElement(BoundCollectionExpression expr, BoundNode element)
private IOperation CreateBoundCollectionExpressionElement(BoundNode element)
{
switch (expr.CollectionTypeKind)
if (element is BoundCollectionExpressionSpreadElement spreadElement)
{
case CollectionExpressionTypeKind.ImplementsIEnumerable:
case CollectionExpressionTypeKind.ImplementsIEnumerableT:
return element switch
{
BoundCollectionExpressionSpreadElement spreadElement => CreateBoundCollectionExpressionSpreadElement(spreadElement, getAddArgument(getIteratorBody(spreadElement))),
BoundCollectionElementInitializer collectionInitializer => Create(collectionInitializer.Arguments[0]),
BoundDynamicCollectionElementInitializer dynamicInitializer => Create(dynamicInitializer.Arguments[0]),
_ => Create(element),
};
default:
return element switch
{
BoundCollectionExpressionSpreadElement spreadElement => CreateBoundCollectionExpressionSpreadElement(spreadElement, getIteratorBody(spreadElement)),
_ => Create(element),
};
}

static BoundExpression? getIteratorBody(BoundCollectionExpressionSpreadElement spreadElement)
{
return ((BoundExpressionStatement?)spreadElement.IteratorBody)?.Expression;
var iteratorBody = ((BoundExpressionStatement?)spreadElement.IteratorBody)?.Expression;
return CreateBoundCollectionExpressionSpreadElement(spreadElement, getUnderlyingExpression(iteratorBody));
}
return Create(getUnderlyingExpression((BoundExpression)element));

[return: NotNullIfNotNull("expr")] static BoundExpression? getAddArgument(BoundExpression? expr)
[return: NotNullIfNotNull("element")] static BoundExpression? getUnderlyingExpression(BoundExpression? element)
{
return expr switch
return element switch
{
// PROTOTYPE: This is not robust. What if there is no applicable Add() method and
// the expression is actually an invocation, and perhaps with no arguments?
BoundCall call => call.Arguments[0],
BoundDynamicInvocation dynamicInvocation => dynamicInvocation.Arguments[0],
// PROTOTYPE: We should look for specific BoundExpression kinds rather than just allowing any.
_ => expr,
BoundCollectionElementInitializer collectionInitializer => collectionInitializer.Arguments[collectionInitializer.InvokedAsExtensionMethod ? 1 : 0],
BoundDynamicCollectionElementInitializer dynamicInitializer => dynamicInitializer.Arguments[0],
_ => element,
};
}
}
Expand Down
Loading

0 comments on commit 5ae2a11

Please sign in to comment.