Skip to content

Commit

Permalink
Support binary interpolation addition (#55059)
Browse files Browse the repository at this point in the history
* Support binding binary operators of interpolated strings as if they were a single interpolated string.
  • Loading branch information
333fred committed Jul 30, 2021
1 parent a7a3315 commit 5e4c37a
Show file tree
Hide file tree
Showing 26 changed files with 2,226 additions and 763 deletions.
87 changes: 62 additions & 25 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1262,13 +1262,9 @@ private bool IsBadBaseAccess(SyntaxNode node, BoundExpression receiverOpt, Symbo
}

internal static uint GetInterpolatedStringHandlerConversionEscapeScope(
BoundInterpolatedString interpolatedString,
InterpolatedStringHandlerData data,
uint scopeOfTheContainingExpression)
{
Debug.Assert(interpolatedString.InterpolationData != null);
var data = interpolatedString.InterpolationData.GetValueOrDefault();
return GetValEscape(data.Construction, scopeOfTheContainingExpression);
}
=> GetValEscape(data.Construction, scopeOfTheContainingExpression);

/// <summary>
/// Computes the scope to which the given invocation can escape
Expand Down Expand Up @@ -2672,7 +2668,13 @@ internal static uint GetValEscape(BoundExpression expr, uint scopeOfTheContainin

if (conversion.ConversionKind == ConversionKind.InterpolatedStringHandler)
{
return GetInterpolatedStringHandlerConversionEscapeScope((BoundInterpolatedString)conversion.Operand, scopeOfTheContainingExpression);
var data = conversion.Operand switch
{
BoundInterpolatedString { InterpolationData: { } d } => d,
BoundBinaryOperator { InterpolatedStringHandlerData: { } d } => d,
_ => throw ExceptionUtilities.UnexpectedValue(conversion.Operand.Kind)
};
return GetInterpolatedStringHandlerConversionEscapeScope(data, scopeOfTheContainingExpression);
}

return GetValEscape(conversion.Operand, scopeOfTheContainingExpression);
Expand Down Expand Up @@ -3110,7 +3112,7 @@ internal static bool CheckValEscape(SyntaxNode node, BoundExpression expr, uint

if (conversion.ConversionKind == ConversionKind.InterpolatedStringHandler)
{
return CheckInterpolatedStringHandlerConversionEscape((BoundInterpolatedString)conversion.Operand, escapeFrom, escapeTo, diagnostics);
return CheckInterpolatedStringHandlerConversionEscape(conversion.Operand, escapeFrom, escapeTo, diagnostics);
}

return CheckValEscape(node, conversion.Operand, escapeFrom, escapeTo, checkingReceiver: false, diagnostics: diagnostics);
Expand Down Expand Up @@ -3383,38 +3385,73 @@ private static bool CheckValEscape(ImmutableArray<BoundExpression> expressions,
return true;
}

private static bool CheckInterpolatedStringHandlerConversionEscape(BoundInterpolatedString interpolatedString, uint escapeFrom, uint escapeTo, BindingDiagnosticBag diagnostics)
private static bool CheckInterpolatedStringHandlerConversionEscape(BoundExpression expression, uint escapeFrom, uint escapeTo, BindingDiagnosticBag diagnostics)
{
Debug.Assert(interpolatedString.InterpolationData is not null);

var data = expression switch
{
BoundInterpolatedString { InterpolationData: { } d } => d,
BoundBinaryOperator { InterpolatedStringHandlerData: { } d } => d,
_ => throw ExceptionUtilities.UnexpectedValue(expression.Kind)
};

// We need to check to see if any values could potentially escape outside the max depth via the handler type.
// Consider the case where a ref-struct handler saves off the result of one call to AppendFormatted,
// and then on a subsequent call it either assigns that saved value to another ref struct with a larger
// escape, or does the opposite. In either case, we need to check.

CheckValEscape(interpolatedString.Syntax, interpolatedString.InterpolationData.GetValueOrDefault().Construction, escapeFrom, escapeTo, checkingReceiver: false, diagnostics);
CheckValEscape(expression.Syntax, data.Construction, escapeFrom, escapeTo, checkingReceiver: false, diagnostics);

foreach (var part in interpolatedString.Parts)
while (true)
{
if (part is not BoundCall { Method: { Name: "AppendFormatted" } } call)
switch (expression)
{
// Dynamic calls cannot have ref struct parameters, and AppendLiteral calls will always have literal
// string arguments and do not require us to be concerned with escape
continue;
}
case BoundBinaryOperator binary:
if (!checkParts((BoundInterpolatedString)binary.Right))
{
return false;
}

// The interpolation component is always the first argument to the method, and it was not passed by name
// so there can be no reordering.
var argument = call.Arguments[0];
var success = CheckValEscape(argument.Syntax, argument, escapeFrom, escapeTo, checkingReceiver: false, diagnostics);
expression = binary.Left;
continue;

if (!success)
{
return false;
case BoundInterpolatedString interpolatedString:
if (!checkParts(interpolatedString))
{
return false;
}

return true;

default:
throw ExceptionUtilities.UnexpectedValue(expression.Kind);
}
}

return true;
bool checkParts(BoundInterpolatedString interpolatedString)
{
foreach (var part in interpolatedString.Parts)
{
if (part is not BoundCall { Method: { Name: "AppendFormatted" } } call)
{
// Dynamic calls cannot have ref struct parameters, and AppendLiteral calls will always have literal
// string arguments and do not require us to be concerned with escape
continue;
}

// The interpolation component is always the first argument to the method, and it was not passed by name
// so there can be no reordering.
var argument = call.Arguments[0];
var success = CheckValEscape(argument.Syntax, argument, escapeFrom, escapeTo, checkingReceiver: false, diagnostics);

if (!success)
{
return false;
}
}

return true;
}
}

internal enum AddressKind
Expand Down
3 changes: 1 addition & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,9 @@ protected BoundExpression CreateConversion(

if (conversion.Kind == ConversionKind.InterpolatedStringHandler)
{
var unconvertedSource = (BoundUnconvertedInterpolatedString)source;
return new BoundConversion(
syntax,
BindUnconvertedInterpolatedStringToHandlerType(unconvertedSource, (NamedTypeSymbol)destination, diagnostics, isHandlerConversion: true),
BindUnconvertedInterpolatedExpressionToHandlerType(source, (NamedTypeSymbol)destination, diagnostics),
conversion,
@checked: CheckOverflowAtRuntime,
explicitCastInCode: isCast && !wasCompilerGenerated,
Expand Down
9 changes: 7 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,11 @@ internal BoundExpression BindToNaturalType(BoundExpression expression, BindingDi
result = BindUnconvertedInterpolatedStringToString(unconvertedInterpolatedString, diagnostics);
}
break;
case BoundBinaryOperator unconvertedBinaryOperator:
{
result = RebindSimpleBinaryOperatorAsConverted(unconvertedBinaryOperator, diagnostics);
}
break;
default:
result = expression;
break;
Expand Down Expand Up @@ -2999,10 +3004,10 @@ private void CoerceArguments<TMember>(

if (kind.IsInterpolatedStringHandler)
{
Debug.Assert(argument is BoundUnconvertedInterpolatedString);
Debug.Assert(argument is BoundUnconvertedInterpolatedString or BoundBinaryOperator { IsUnconvertedInterpolatedStringAddition: true });
TypeWithAnnotations parameterTypeWithAnnotations = GetCorrespondingParameterTypeWithAnnotations(ref result, parameters, arg);
reportUnsafeIfNeeded(methodResult, diagnostics, argument, parameterTypeWithAnnotations);
arguments[arg] = BindInterpolatedStringHandlerInMemberCall((BoundUnconvertedInterpolatedString)argument, arguments, parameters, ref result, arg, receiverType, receiverRefKind, receiverEscapeScope, diagnostics);
arguments[arg] = BindInterpolatedStringHandlerInMemberCall(argument, arguments, parameters, ref result, arg, receiverType, receiverRefKind, receiverEscapeScope, diagnostics);
}
// https://github.com/dotnet/roslyn/issues/37119 : should we create an (Identity) conversion when the kind is Identity but the types differ?
else if (!kind.IsIdentity)
Expand Down
Loading

0 comments on commit 5e4c37a

Please sign in to comment.