Skip to content

Commit

Permalink
Add support for all parenthesized binary additions of interpolated st…
Browse files Browse the repository at this point in the history 
…rings

Implements the decision from dotnet/csharplang#5106. Test plan at dotnet#51499.
  • Loading branch information
@333fred
333fred committed Sep 11, 2021
1 parent 2d08d2c commit f1e54a5
Show file tree
Hide file tree
Showing 8 changed files with 285 additions and 142 deletions.
179 changes: 92 additions & 87 deletions src/Compilers/CSharp/Portable/Binder/Binder_InterpolatedString.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,71 +230,36 @@ private bool TryBindUnconvertedBinaryOperatorToDefaultInterpolatedStringHandler(
return false;
}

bool isConstant = true;
var stack = ArrayBuilder<BoundBinaryOperator>.GetInstance();
// The constant value is folded as part of creating the unconverted operator. If there is a constant value, then the top-level binary operator
// will have one.
bool isConstant = binaryOperator.ConstantValue is not null;
var partsArrayBuilder = ArrayBuilder<ImmutableArray<BoundExpression>>.GetInstance();
int partsCount = 0;

BoundBinaryOperator? current = binaryOperator;

while (current != null)
{
Debug.Assert(current.IsUnconvertedInterpolatedStringAddition);
stack.Push(current);
isConstant = isConstant && current.Right.ConstantValue is not null;
var rightInterpolatedString = (BoundUnconvertedInterpolatedString)current.Right;

if (!ValidateInterpolatedStringParts(rightInterpolatedString))
{
// Exception to case 3. Delegate to standard binding.
stack.Free();
partsArrayBuilder.Free();
return false;
}

partsCount += rightInterpolatedString.Parts.Length;
partsArrayBuilder.Add(rightInterpolatedString.Parts);

switch (current.Left)
if (!binaryOperator.VisitBinaryOperatorInterpolatedString(partsArrayBuilder, static (BoundUnconvertedInterpolatedString unconvertedInterpolatedString, ArrayBuilder<ImmutableArray<BoundExpression>> partsArrayBuilder) =>
{
case BoundBinaryOperator leftOperator:
current = leftOperator;
continue;
case BoundUnconvertedInterpolatedString interpolatedString:
isConstant = isConstant && interpolatedString.ConstantValue is not null;

if (!ValidateInterpolatedStringParts(interpolatedString))
{
// Exception to case 3. Delegate to standard binding.
stack.Free();
partsArrayBuilder.Free();
return false;
}
if (!ValidateInterpolatedStringParts(unconvertedInterpolatedString))
{
return false;
}

partsCount += interpolatedString.Parts.Length;
partsArrayBuilder.Add(interpolatedString.Parts);
current = null;
break;
default:
throw ExceptionUtilities.UnexpectedValue(current.Left.Kind);
}
partsArrayBuilder.Add(unconvertedInterpolatedString.Parts);
return true;
}))
{
partsArrayBuilder.Free();
return false;
}

Debug.Assert(partsArrayBuilder.Count == stack.Count + 1);
Debug.Assert(partsArrayBuilder.Count >= 2);

if (isConstant ||
(partsCount <= 4 && partsArrayBuilder.All(static parts => AllInterpolatedStringPartsAreStrings(parts))))
(partsArrayBuilder.Count <= 4 && partsArrayBuilder.All(static parts => AllInterpolatedStringPartsAreStrings(parts))))
{
// This is case 1 and 2. Let the standard machinery handle it
stack.Free();
partsArrayBuilder.Free();
return false;
}

// Parts were added to the array from right to left, but lexical order is left to right.
partsArrayBuilder.ReverseContents();

// Case 3. Bind as handler.
var (appendCalls, data) = BindUnconvertedInterpolatedPartsToHandlerType(
binaryOperator.Syntax,
Expand All @@ -306,32 +271,81 @@ private bool TryBindUnconvertedBinaryOperatorToDefaultInterpolatedStringHandler(
additionalConstructorRefKinds: default);

// Now that the parts have been bound, reconstruct the binary operators.
convertedBinaryOperator = UpdateBinaryOperatorWithInterpolatedContents(stack, appendCalls, data, binaryOperator.Syntax, diagnostics);
stack.Free();
convertedBinaryOperator = UpdateBinaryOperatorWithInterpolatedContents(binaryOperator, appendCalls, data, binaryOperator.Syntax, diagnostics);
return true;
}

private BoundBinaryOperator UpdateBinaryOperatorWithInterpolatedContents(ArrayBuilder<BoundBinaryOperator> stack, ImmutableArray<ImmutableArray<BoundExpression>> appendCalls, InterpolatedStringHandlerData data, SyntaxNode rootSyntax, BindingDiagnosticBag diagnostics)
private BoundBinaryOperator UpdateBinaryOperatorWithInterpolatedContents(BoundBinaryOperator originalOperator, ImmutableArray<ImmutableArray<BoundExpression>> appendCalls, InterpolatedStringHandlerData data, SyntaxNode rootSyntax, BindingDiagnosticBag diagnostics)
{
Debug.Assert(appendCalls.Length == stack.Count + 1);
var @string = GetSpecialType(SpecialType.System_String, diagnostics, rootSyntax);
var originalStack = ArrayBuilder<BoundBinaryOperator>.GetInstance();
var rewrittenLefts = ArrayBuilder<(BoundExpression original, BoundExpression rewritten)>.GetInstance();
(BoundBinaryOperator? original, BoundBinaryOperator? rewritten) result = default;

var bottomOperator = stack.Pop();
var result = createBinaryOperator(bottomOperator, createInterpolation(bottomOperator.Left, appendCalls[0]), rightIndex: 1);
pushLeftNodes(originalOperator, originalStack);
var @string = GetSpecialType(SpecialType.System_String, diagnostics, rootSyntax);

for (int i = 2; i < appendCalls.Length; i++)
int i = 0;
while (originalStack.TryPeek(out var currentBinary))
{
result = createBinaryOperator(stack.Pop(), result, rightIndex: i);
Debug.Assert(currentBinary.Left is BoundUnconvertedInterpolatedString || rewrittenLefts.Count != 0 || currentBinary.Left == result.original);

if (currentBinary.Left is BoundUnconvertedInterpolatedString originalLeft)
{
if (rewrittenLefts.TryPeek(out var rewrittenLeftTuple) && rewrittenLeftTuple.original == originalLeft)
{
// Leave it alone, we've already rewritten on the first pass.
Debug.Assert(currentBinary.Right.Kind == BoundKind.BinaryOperator);
}
else
{
Debug.Assert(appendCalls.Length > i);
rewrittenLefts.Push((originalLeft, createInterpolation(originalLeft, appendCalls[i++])));
}
}
else if (currentBinary.Left == result.original)
{
Debug.Assert(result.rewritten != null);
rewrittenLefts.Push((currentBinary.Left, result.rewritten));
}

switch (currentBinary.Right)
{
case BoundUnconvertedInterpolatedString originalRightString:
Debug.Assert(appendCalls.Length > i);
var rewrittenLeft = rewrittenLefts.Pop().rewritten;
BoundExpression rewrittenRight = createInterpolation(originalRightString, appendCalls[i++]);
result = (currentBinary, createBinaryOperator(currentBinary, rewrittenLeft, rewrittenRight));
originalStack.Pop();
break;

case BoundBinaryOperator originalRightOperator when result.original == originalRightOperator:
// If result.original is originalRightOperator, then this is the second time we're visiting
// this node and can rewrite it. Otherwise, we need to push all the left nodes, visit them,
// then come back again.
Debug.Assert(result.rewritten != null);
rewrittenLeft = rewrittenLefts.Pop().rewritten;
rewrittenRight = result.rewritten;
result = (currentBinary, createBinaryOperator(currentBinary, rewrittenLeft, rewrittenRight));
originalStack.Pop();
break;

case BoundBinaryOperator originalRightOperator:
pushLeftNodes(originalRightOperator, originalStack);
break;
}
}

return result.Update(BoundBinaryOperator.UncommonData.InterpolatedStringHandlerAddition(data));
Debug.Assert(result.rewritten != null);
Debug.Assert(i == appendCalls.Length);

return result.rewritten.Update(BoundBinaryOperator.UncommonData.InterpolatedStringHandlerAddition(data));

BoundBinaryOperator createBinaryOperator(BoundBinaryOperator original, BoundExpression left, int rightIndex)
BoundBinaryOperator createBinaryOperator(BoundBinaryOperator original, BoundExpression left, BoundExpression right)
=> new BoundBinaryOperator(
original.Syntax,
BinaryOperatorKind.StringConcatenation,
left,
createInterpolation(original.Right, appendCalls[rightIndex]),
right,
original.ConstantValue,
methodOpt: null,
constrainedToTypeOpt: null,
Expand All @@ -351,6 +365,17 @@ static BoundInterpolatedString createInterpolation(BoundExpression expression, I
expression.Type,
expression.HasErrors);
}

static void pushLeftNodes(BoundBinaryOperator binary, ArrayBuilder<BoundBinaryOperator> stack)
{
BoundBinaryOperator? current = binary;
while (current != null)
{
Debug.Assert(current.IsUnconvertedInterpolatedStringAddition);
stack.Push(current);
current = current.Left as BoundBinaryOperator;
}
}
}

private BoundExpression BindUnconvertedInterpolatedExpressionToHandlerType(
Expand Down Expand Up @@ -408,32 +433,13 @@ private BoundBinaryOperator BindUnconvertedBinaryOperatorToInterpolatedStringHan
{
Debug.Assert(binaryOperator.IsUnconvertedInterpolatedStringAddition);

var stack = ArrayBuilder<BoundBinaryOperator>.GetInstance();
var partsArrayBuilder = ArrayBuilder<ImmutableArray<BoundExpression>>.GetInstance();

BoundBinaryOperator? current = binaryOperator;

while (current != null)
binaryOperator.VisitBinaryOperatorInterpolatedString(partsArrayBuilder, (BoundUnconvertedInterpolatedString unconvertedInterpolatedString, ArrayBuilder<ImmutableArray<BoundExpression>> partsArrayBuilder) =>
{
stack.Push(current);
partsArrayBuilder.Add(((BoundUnconvertedInterpolatedString)current.Right).Parts);

if (current.Left is BoundBinaryOperator next)
{
current = next;
}
else
{
partsArrayBuilder.Add(((BoundUnconvertedInterpolatedString)current.Left).Parts);
current = null;
}
}

// Parts are added in right to left order, but lexical is left to right.
partsArrayBuilder.ReverseContents();

Debug.Assert(partsArrayBuilder.Count == stack.Count + 1);
Debug.Assert(partsArrayBuilder.Count >= 2);
partsArrayBuilder.Add(unconvertedInterpolatedString.Parts);
return true;
});

var (appendCalls, data) = BindUnconvertedInterpolatedPartsToHandlerType(
binaryOperator.Syntax,
Expand All @@ -444,8 +450,7 @@ private BoundBinaryOperator BindUnconvertedBinaryOperatorToInterpolatedStringHan
additionalConstructorArguments,
additionalConstructorRefKinds);

var result = UpdateBinaryOperatorWithInterpolatedContents(stack, appendCalls, data, binaryOperator.Syntax, diagnostics);
stack.Free();
var result = UpdateBinaryOperatorWithInterpolatedContents(binaryOperator, appendCalls, data, binaryOperator.Syntax, diagnostics);
return result;
}

Expand Down
9 changes: 7 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ private BoundExpression BindSimpleBinaryOperator(BinaryExpressionSyntax node, Bi
if (leaveUnconvertedIfInterpolatedString
&& kind == BinaryOperatorKind.Addition
&& left is BoundUnconvertedInterpolatedString or BoundBinaryOperator { IsUnconvertedInterpolatedStringAddition: true }
&& right is BoundUnconvertedInterpolatedString)
&& right is BoundUnconvertedInterpolatedString or BoundBinaryOperator { IsUnconvertedInterpolatedStringAddition: true })
{
Debug.Assert(right.Type.SpecialType == SpecialType.System_String);
var stringConstant = FoldBinaryOperator(node, BinaryOperatorKind.StringConcatenation, left, right, right.Type, diagnostics);
Expand Down Expand Up @@ -727,7 +727,12 @@ private BoundExpression RebindSimpleBinaryOperatorAsConverted(BoundBinaryOperato
BoundExpression? left = null;
while (stack.TryPop(out current))
{
Debug.Assert(current.Right is BoundUnconvertedInterpolatedString);
var right = current.Right switch
{
BoundUnconvertedInterpolatedString s => s,
BoundBinaryOperator b => RebindSimpleBinaryOperatorAsConverted(b, diagnostics),
_ => throw ExceptionUtilities.UnexpectedValue(current.Right.Kind)
};
left = BindSimpleBinaryOperator((BinaryExpressionSyntax)current.Syntax, diagnostics, left ?? current.Left, current.Right, leaveUnconvertedIfInterpolatedString: false);
}

Expand Down
65 changes: 65 additions & 0 deletions src/Compilers/CSharp/Portable/BoundTree/BoundNodeExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@
// 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;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp
{
Expand Down Expand Up @@ -94,5 +97,67 @@ private class ContainsAwaitVisitor : BoundTreeWalkerWithStackGuardWithoutRecursi
return null;
}
}

public static bool VisitBinaryOperatorInterpolatedString<TArg, TInterpolatedStringType>(
this BoundBinaryOperator binary,
TArg arg,
Func<TInterpolatedStringType, TArg, bool> visitor,
Action<BoundBinaryOperator, TArg>? binaryOperatorCallback = null)
where TInterpolatedStringType : BoundExpression
{
Debug.Assert(typeof(TInterpolatedStringType) == typeof(BoundUnconvertedInterpolatedString) || typeof(TInterpolatedStringType) == typeof(BoundInterpolatedString));
var stack = ArrayBuilder<BoundBinaryOperator>.GetInstance();

pushLeftNodes(binary, stack);

while (stack.TryPop(out BoundBinaryOperator? current))
{
switch (current.Left)
{
case BoundBinaryOperator op:
binaryOperatorCallback?.Invoke(op, arg);
break;
case TInterpolatedStringType interpolatedString:
if (!visitor(interpolatedString, arg))
{
return false;
}
break;
default:
throw ExceptionUtilities.UnexpectedValue(current.Left.Kind);
}

switch (current.Right)
{
case BoundBinaryOperator rightOperator:
binaryOperatorCallback?.Invoke(rightOperator, arg);
pushLeftNodes(rightOperator, stack);
break;
case TInterpolatedStringType interpolatedString:
if (!visitor(interpolatedString, arg))
{
return false;
}
break;
default:
throw ExceptionUtilities.UnexpectedValue(current.Right.Kind);
}
}

Debug.Assert(stack.Count == 0);
stack.Free();
return true;

static void pushLeftNodes(BoundBinaryOperator binary, ArrayBuilder<BoundBinaryOperator> stack)
{
Debug.Assert(typeof(TInterpolatedStringType) == typeof(BoundInterpolatedString) || binary.IsUnconvertedInterpolatedStringAddition);
BoundBinaryOperator? current = binary;
while (current != null)
{
stack.Push(current);
current = current.Left as BoundBinaryOperator;
}
}
}
}
}
27 changes: 9 additions & 18 deletions src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2505,10 +2505,14 @@ protected void VisitBinaryInterpolatedStringAddition(BoundBinaryOperator node)
var stack = ArrayBuilder<BoundInterpolatedString>.GetInstance();
var data = node.InterpolatedStringHandlerData.GetValueOrDefault();

while (PushBinaryOperatorInterpolatedStringChildren(node, stack) is { } next)
{
node = next;
}
node.VisitBinaryOperatorInterpolatedString(
(stack, @this: this),
visitor: (BoundInterpolatedString interpolatedString, (ArrayBuilder<BoundInterpolatedString> stack, AbstractFlowPass<TLocalState, TLocalFunctionState> @this) arg) =>
{
arg.stack.Push(interpolatedString);
return true;
},
binaryOperatorCallback: (op, arg) => arg.@this.OnPushBinaryOperatorInterpolatedStringChildren(op));

Debug.Assert(stack.Count >= 2);

Expand All @@ -2532,20 +2536,7 @@ protected void VisitBinaryInterpolatedStringAddition(BoundBinaryOperator node)
stack.Free();
}

protected virtual BoundBinaryOperator? PushBinaryOperatorInterpolatedStringChildren(BoundBinaryOperator node, ArrayBuilder<BoundInterpolatedString> stack)
{
stack.Push((BoundInterpolatedString)node.Right);
switch (node.Left)
{
case BoundBinaryOperator next:
return next;
case BoundInterpolatedString @string:
stack.Push(@string);
return null;
default:
throw ExceptionUtilities.UnexpectedValue(node.Left.Kind);
}
}
protected virtual void OnPushBinaryOperatorInterpolatedStringChildren(BoundBinaryOperator node) { }

protected virtual bool VisitInterpolatedStringHandlerParts(BoundInterpolatedStringBase node, bool usesBoolReturns, bool firstPartIsConditional, ref TLocalState? shortCircuitState)
{
Expand Down
Loading

0 comments on commit f1e54a5

Please sign in to comment.