Skip to content

Commit

Permalink
Escape rules for deconstruction and foreach variables (#22354)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcouv
jcouv committed Sep 29, 2017
1 parent 1fd9a04 commit 3d3b524
Show file tree
Hide file tree
Showing 10 changed files with 865 additions and 46 deletions.
76 changes: 73 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1954,6 +1954,27 @@ internal static bool CheckRefEscape(SyntaxNode node, BoundExpression expr, uint
return false;
}

internal static uint GetBroadestValEscape(BoundTupleExpression expr, uint scopeOfTheContainingExpression)
{
uint broadest = scopeOfTheContainingExpression;
foreach (var element in expr.Arguments)
{
uint valEscape;
if (element.Kind == BoundKind.TupleLiteral)
{
valEscape = GetBroadestValEscape((BoundTupleExpression)element, scopeOfTheContainingExpression);
}
else
{
valEscape = GetValEscape(element, scopeOfTheContainingExpression);
}

broadest = Math.Min(broadest, valEscape);
}

return broadest;
}

/// <summary>
/// Computes the widest scope depth to which the given expression can escape by value.
///
Expand Down Expand Up @@ -1989,6 +2010,14 @@ internal static uint GetValEscape(BoundExpression expr, uint scopeOfTheContainin
// always returnable
return Binder.ExternalScope;

case BoundKind.TupleLiteral:
var tupleLiteral = (BoundTupleLiteral)expr;
return GetTupleValEscape(tupleLiteral.Arguments, scopeOfTheContainingExpression);

case BoundKind.ConvertedTupleLiteral:
var convertedTupleLiteral = (BoundConvertedTupleLiteral)expr;
return GetTupleValEscape(convertedTupleLiteral.Arguments, scopeOfTheContainingExpression);

case BoundKind.MakeRefOperator:
case BoundKind.RefValueOperator:
// for compat reasons
Expand All @@ -2000,6 +2029,9 @@ internal static uint GetValEscape(BoundExpression expr, uint scopeOfTheContainin
// same as uninitialized local
return Binder.ExternalScope;

case BoundKind.DeconstructValuePlaceholder:
return ((BoundDeconstructValuePlaceholder)expr).ValEscape;

case BoundKind.Local:
return ((BoundLocal)expr).LocalSymbol.ValEscapeScope;

Expand Down Expand Up @@ -2162,6 +2194,17 @@ internal static uint GetValEscape(BoundExpression expr, uint scopeOfTheContainin
}
}

private static uint GetTupleValEscape(ImmutableArray<BoundExpression> elements, uint scopeOfTheContainingExpression)
{
uint narrowestScope = scopeOfTheContainingExpression;
foreach (var element in elements)
{
narrowestScope = Math.Max(narrowestScope, GetValEscape(element, scopeOfTheContainingExpression));
}

return narrowestScope;
}

private static uint GetValEscapeOfObjectInitializer(BoundObjectInitializerExpression initExpr, uint scopeOfTheContainingExpression)
{
var result = Binder.ExternalScope;
Expand Down Expand Up @@ -2236,6 +2279,14 @@ internal static bool CheckValEscape(SyntaxNode node, BoundExpression expr, uint
// always returnable
return true;

case BoundKind.TupleLiteral:
var tupleLiteral = (BoundTupleLiteral)expr;
return CheckTupleValEscape(tupleLiteral.Arguments, escapeFrom, escapeTo, diagnostics);

case BoundKind.ConvertedTupleLiteral:
var convertedTupleLiteral = (BoundConvertedTupleLiteral)expr;
return CheckTupleValEscape(convertedTupleLiteral.Arguments, escapeFrom, escapeTo, diagnostics);

case BoundKind.MakeRefOperator:
case BoundKind.RefValueOperator:
// for compat reasons
Expand All @@ -2245,6 +2296,15 @@ internal static bool CheckValEscape(SyntaxNode node, BoundExpression expr, uint
// same as uninitialized local
return true;

case BoundKind.DeconstructValuePlaceholder:
var placeholder = (BoundDeconstructValuePlaceholder)expr;
if (placeholder.ValEscape > escapeTo)
{
Error(diagnostics, ErrorCode.ERR_EscapeLocal, node, placeholder.Syntax);
return false;
}
return true;

case BoundKind.Local:
var localSymbol = ((BoundLocal)expr).LocalSymbol;
if (localSymbol.ValEscapeScope > escapeTo)
Expand Down Expand Up @@ -2459,8 +2519,6 @@ internal static bool CheckValEscape(SyntaxNode node, BoundExpression expr, uint
// case BoundKind.NameOfOperator:
// case BoundKind.InterpolatedString:
// case BoundKind.StringInsert:
// case BoundKind.TupleLiteral:
// case BoundKind.ConvertedTupleLiteral:
// case BoundKind.DynamicIndexerAccess:
// case BoundKind.Lambda:
// case BoundKind.DynamicObjectCreationExpression:
Expand Down Expand Up @@ -2537,7 +2595,6 @@ internal static bool CheckValEscape(SyntaxNode node, BoundExpression expr, uint
// case BoundKind.DeclarationPattern:
// case BoundKind.ConstantPattern:
// case BoundKind.WildcardPattern:
// case BoundKind.DeconstructValuePlaceholder:

#endregion

Expand Down Expand Up @@ -2573,6 +2630,19 @@ internal static bool CheckValEscape(SyntaxNode node, BoundExpression expr, uint
}
}

private static bool CheckTupleValEscape(ImmutableArray<BoundExpression> elements, uint escapeFrom, uint escapeTo, DiagnosticBag diagnostics)
{
foreach (var element in elements)
{
if (!CheckValEscape(element.Syntax, element, escapeFrom, escapeTo, checkingReceiver: false, diagnostics: diagnostics))
{
return false;
}
}

return true;
}

private static bool CheckValEscapeOfObjectInitializer(BoundObjectInitializerExpression initExpr, uint escapeFrom, uint escapeTo, DiagnosticBag diagnostics)
{
foreach (var expression in initExpr.Initializers)
Expand Down
59 changes: 39 additions & 20 deletions src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.PooledObjects;
Expand Down Expand Up @@ -95,13 +94,15 @@ internal BoundExpression BindDeconstruction(AssignmentExpressionSyntax node, Dia
DeconstructionVariable locals = BindDeconstructionVariables(left, diagnostics, ref declaration, ref expression);
Debug.Assert(locals.HasNestedVariables);

BoundExpression boundRight = rightPlaceholder ?? BindValue(right, diagnostics, BindValueKind.RValue);
boundRight = FixTupleLiteral(locals.NestedVariables, boundRight, deconstruction, diagnostics);
var deconstructionDiagnostics = new DiagnosticBag();
BoundExpression boundRight = rightPlaceholder ?? BindValue(right, deconstructionDiagnostics, BindValueKind.RValue);
boundRight = FixTupleLiteral(locals.NestedVariables, boundRight, deconstruction, deconstructionDiagnostics);

bool resultIsUsed = resultIsUsedOverride || IsDeconstructionResultUsed(left);
var assignment = BindDeconstructionAssignment(deconstruction, left, boundRight, locals.NestedVariables, resultIsUsed, diagnostics);
var assignment = BindDeconstructionAssignment(deconstruction, left, boundRight, locals.NestedVariables, resultIsUsed, deconstructionDiagnostics);
DeconstructionVariable.FreeDeconstructionVariables(locals.NestedVariables);

diagnostics.AddRange(deconstructionDiagnostics);
return assignment;
}

Expand All @@ -113,10 +114,12 @@ internal BoundExpression BindDeconstruction(AssignmentExpressionSyntax node, Dia
bool resultIsUsed,
DiagnosticBag diagnostics)
{
uint rightEscape = GetValEscape(boundRHS, this.LocalScopeDepth);

if ((object)boundRHS.Type == null || boundRHS.Type.IsErrorType())
{
// we could still not infer a type for the RHS
FailRemainingInferences(checkedVariables, diagnostics);
FailRemainingInferencesAndSetValEscape(checkedVariables, diagnostics, rightEscape);
var voidType = GetSpecialType(SpecialType.System_Void, diagnostics, node);

var type = boundRHS.Type ?? voidType;
Expand All @@ -139,11 +142,14 @@ internal BoundExpression BindDeconstruction(AssignmentExpressionSyntax node, Dia
checkedVariables,
out conversion);

FailRemainingInferences(checkedVariables, diagnostics);
FailRemainingInferencesAndSetValEscape(checkedVariables, diagnostics, rightEscape);

var lhsTuple = DeconstructionVariablesAsTuple(left, checkedVariables, diagnostics, ignoreDiagnosticsFromTuple: hasErrors || !resultIsUsed);
var lhsTuple = DeconstructionVariablesAsTuple(left, checkedVariables, diagnostics, ignoreDiagnosticsFromTuple: diagnostics.HasAnyErrors() || !resultIsUsed);
TypeSymbol returnType = hasErrors ? CreateErrorType() : lhsTuple.Type;

uint leftEscape = GetBroadestValEscape(lhsTuple, this.LocalScopeDepth);
boundRHS = ValidateEscape(boundRHS, leftEscape, isByRef: false, diagnostics: diagnostics);

var boundConversion = new BoundConversion(
boundRHS.Syntax,
boundRHS,
Expand Down Expand Up @@ -197,7 +203,10 @@ private BoundExpression FixTupleLiteral(ArrayBuilder<DeconstructionVariable> che
// Let's fix the literal up by figuring out its type
// For declarations, that means merging type information from the LHS and RHS
// For assignments, only the LHS side matters since it is necessarily typed
TypeSymbol mergedTupleType = MakeMergedTupleType(checkedVariables, (BoundTupleLiteral)boundRHS, syntax, Compilation, diagnostics);

// If we already have diagnostics at this point, it is not worth collecting likely duplicate diagnostics from making the merged type
bool hadErrors = diagnostics.HasAnyErrors();
TypeSymbol mergedTupleType = MakeMergedTupleType(checkedVariables, (BoundTupleLiteral)boundRHS, syntax, Compilation, hadErrors ? null : diagnostics);
if ((object)mergedTupleType != null)
{
boundRHS = GenerateConversionForAssignment(mergedTupleType, boundRHS, diagnostics);
Expand Down Expand Up @@ -248,7 +257,7 @@ private BoundExpression FixTupleLiteral(ArrayBuilder<DeconstructionVariable> che
else
{
ImmutableArray<BoundDeconstructValuePlaceholder> outPlaceholders;
var inputPlaceholder = new BoundDeconstructValuePlaceholder(syntax, type);
var inputPlaceholder = new BoundDeconstructValuePlaceholder(syntax, this.LocalScopeDepth, type);
var deconstructInvocation = MakeDeconstructInvocationExpression(variables.Count,
inputPlaceholder, rightSyntax, diagnostics, out outPlaceholders);

Expand Down Expand Up @@ -345,24 +354,33 @@ private BoundExpression SetInferredType(BoundExpression expression, TypeSymbol t

/// <summary>
/// Find any deconstruction locals that are still pending inference and fail their inference.
/// Set the safe-to-escape scope for all deconstruction locals.
/// </summary>
private void FailRemainingInferences(ArrayBuilder<DeconstructionVariable> variables, DiagnosticBag diagnostics)
private void FailRemainingInferencesAndSetValEscape(ArrayBuilder<DeconstructionVariable> variables, DiagnosticBag diagnostics,
uint rhsValEscape)
{
int count = variables.Count;
for (int i = 0; i < count; i++)
{
var variable = variables[i];
if (variable.HasNestedVariables)
{
FailRemainingInferences(variable.NestedVariables, diagnostics);
FailRemainingInferencesAndSetValEscape(variable.NestedVariables, diagnostics, rhsValEscape);
}
else
{
switch (variable.Single.Kind)
{
case BoundKind.Local:
var local = (BoundLocal)variable.Single;
if (local.IsDeclaration)
{
((SourceLocalSymbol)local.LocalSymbol).SetValEscape(rhsValEscape);
}
break;
case BoundKind.DeconstructionVariablePendingInference:
BoundExpression local = ((DeconstructionVariablePendingInference)variable.Single).FailInference(this, diagnostics);
variables[i] = new DeconstructionVariable(local, local.Syntax);
BoundExpression errorLocal = ((DeconstructionVariablePendingInference)variable.Single).FailInference(this, diagnostics);
variables[i] = new DeconstructionVariable(errorLocal, errorLocal.Syntax);
break;
case BoundKind.DiscardExpression:
var pending = (BoundDiscardExpression)variable.Single;
Expand Down Expand Up @@ -439,6 +457,7 @@ private static TypeSymbol MakeMergedTupleType(ArrayBuilder<DeconstructionVariabl
int rightLength = rhsLiteral.Arguments.Length;

var typesBuilder = ArrayBuilder<TypeSymbol>.GetInstance(leftLength);
var locationsBuilder = ArrayBuilder<Location>.GetInstance(leftLength);
for (int i = 0; i < rightLength; i++)
{
BoundExpression element = rhsLiteral.Arguments[i];
Expand Down Expand Up @@ -479,29 +498,29 @@ private static TypeSymbol MakeMergedTupleType(ArrayBuilder<DeconstructionVariabl
}

typesBuilder.Add(mergedType);
locationsBuilder.Add(element.Syntax.Location);
}

if (typesBuilder.Any(t => t == null))
{
typesBuilder.Free();
locationsBuilder.Free();
return null;
}

// The tuple created here is not identical to the one created by
// DeconstructionVariablesAsTuple. It represents a smaller
// tree of types used for figuring out natural types in tuple literal.
// Therefore, we do not check constraints here as it would report errors
// that are already reported later. DeconstructionVariablesAsTuple
// constructs the final tuple type and checks constraints.
return TupleTypeSymbol.Create(
locationOpt: null,
elementTypes: typesBuilder.ToImmutableAndFree(),
elementLocations: default(ImmutableArray<Location>),
elementLocations: locationsBuilder.ToImmutableAndFree(),
elementNames: default(ImmutableArray<string>),
compilation: compilation,
diagnostics: diagnostics,
shouldCheckConstraints: false,
errorPositions: default(ImmutableArray<bool>));
shouldCheckConstraints: true,
errorPositions: default(ImmutableArray<bool>),
syntax: syntax);
}

private BoundTupleLiteral DeconstructionVariablesAsTuple(CSharpSyntaxNode syntax, ArrayBuilder<DeconstructionVariable> variables,
Expand Down Expand Up @@ -672,7 +691,7 @@ private static string ExtractDeconstructResultElementName(BoundExpression expres
out ImmutableArray<BoundDeconstructValuePlaceholder> outPlaceholders, BoundExpression childNode)
{
Error(diagnostics, ErrorCode.ERR_MissingDeconstruct, rightSyntax, receiver.Type, numParameters);
outPlaceholders = default(ImmutableArray<BoundDeconstructValuePlaceholder>);
outPlaceholders = default;

return BadExpression(rightSyntax, childNode);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2536,7 +2536,7 @@ private BoundExpression BindArgumentExpression(DiagnosticBag diagnostics, Expres
else if (argument.Kind == BoundKind.OutDeconstructVarPendingInference)
{
TypeSymbol parameterType = GetCorrespondingParameterType(ref result, parameters, arg);
arguments[arg] = ((OutDeconstructVarPendingInference)argument).SetInferredType(parameterType, success: true);
arguments[arg] = ((OutDeconstructVarPendingInference)argument).SetInferredType(parameterType, this, success: true);
}
else if (argument.Kind == BoundKind.DiscardExpression && !argument.HasExpressionType())
{
Expand Down
Loading

0 comments on commit 3d3b524

Please sign in to comment.