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.

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

Enable out variables in indexers, and implement lazy inference of pattern variables. #13274

Merged
merged 14 commits into from
Aug 30, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
37 changes: 21 additions & 16 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1242,22 +1242,7 @@ private BoundExpression BindNonMethod(SimpleNameSyntax node, Symbol symbol, Diag
Location localSymbolLocation = localSymbol.Locations[0];

TypeSymbol type;
if ((localSymbol as SourceLocalSymbol)?.IsVar == true && localSymbol.ForbiddenZone?.Contains(node) == true)
{
// A var (type-inferred) local variable has been used in its own initialization (the "forbidden zone").
// There are many cases where this occurs, including:
//
// 1. var x = M(out x);
// 2. M(out var x, out x);
// 3. var (x, y) = (y, x);
//
// localSymbol.ForbiddenDiagnostic provides a suitable diagnostic for whichever case applies.
//
diagnostics.Add(localSymbol.ForbiddenDiagnostic, node.Location, node);
type = new ExtendedErrorTypeSymbol(
this.Compilation, name: "var", arity: 0, errorInfo: null, variableUsedBeforeDeclaration: true);
}
else if (node.SyntaxTree == localSymbolLocation.SourceTree &&
if (node.SyntaxTree == localSymbolLocation.SourceTree &&
node.SpanStart < localSymbolLocation.SourceSpan.Start)
{
// Here we report a local variable being used before its declaration
Expand Down Expand Up @@ -1320,6 +1305,21 @@ private BoundExpression BindNonMethod(SimpleNameSyntax node, Symbol symbol, Diag
type = new ExtendedErrorTypeSymbol(
this.Compilation, name: "var", arity: 0, errorInfo: null, variableUsedBeforeDeclaration: true);
}
else if ((localSymbol as SourceLocalSymbol)?.IsVar == true && localSymbol.ForbiddenZone?.Contains(node) == true)
{
// A var (type-inferred) local variable has been used in its own initialization (the "forbidden zone").
// There are many cases where this occurs, including:
//
// 1. var x = M(out x);
// 2. M(out var x, out x);
// 3. var (x, y) = (y, x);
//
// localSymbol.ForbiddenDiagnostic provides a suitable diagnostic for whichever case applies.
//
diagnostics.Add(localSymbol.ForbiddenDiagnostic, node.Location, node);
type = new ExtendedErrorTypeSymbol(
this.Compilation, name: "var", arity: 0, errorInfo: null, variableUsedBeforeDeclaration: true);
}
else
{
type = localSymbol.Type;
Expand Down Expand Up @@ -6060,6 +6060,11 @@ private BoundExpression ConvertToArrayIndex(BoundExpression index, SyntaxNode no
{
Debug.Assert(index != null);

if (index.Kind == BoundKind.OutVarLocalPendingInference)
{
return ((OutVarLocalPendingInference)index).FailInference(this, diagnostics);
}

var result =
TryImplicitConversionToArrayIndex(index, SpecialType.System_Int32, node, diagnostics) ??
TryImplicitConversionToArrayIndex(index, SpecialType.System_UInt32, node, diagnostics) ??
Expand Down
22 changes: 6 additions & 16 deletions src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs
Original file line number Diff line number Diff line change
Expand Up @@ -398,40 +398,30 @@ private BoundExpression BindSimpleBinaryOperator(BinaryExpressionSyntax node, Di

Debug.Assert(IsSimpleBinaryOperator(node.Kind()));

var expressions = ArrayBuilder<BoundExpression>.GetInstance();
var syntaxNodes = ArrayBuilder<BinaryExpressionSyntax>.GetInstance();

ExpressionSyntax current = node;
while (IsSimpleBinaryOperator(current.Kind()))
{
var binOp = (BinaryExpressionSyntax)current;
syntaxNodes.Push(binOp);
expressions.Push(BindValue(binOp.Right, diagnostics, BindValueKind.RValue));
current = binOp.Left;
}
BoundExpression leftMost = BindExpression(current, diagnostics);
expressions.Push(leftMost);

Debug.Assert(syntaxNodes.Count + 1 == expressions.Count);
int compoundStringLength = 0;

BoundExpression result = BindExpression(current, diagnostics);
while (syntaxNodes.Count > 0)
{
BinaryExpressionSyntax syntaxNode = syntaxNodes.Pop();
BindValueKind bindValueKind = GetBinaryAssignmentKind(syntaxNode.Kind());
BoundExpression left = expressions.Pop();
BoundExpression right = expressions.Pop();
left = CheckValue(left, bindValueKind, diagnostics);
BoundExpression left = CheckValue(result, bindValueKind, diagnostics);
BoundExpression right = BindValue(syntaxNode.Right, diagnostics, BindValueKind.RValue);
Copy link
Member Author

@gafter gafter Aug 24, 2016

Choose a reason for hiding this comment

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

BindValue [](start = 40, length = 9)

The purpose of this change is to cause binary expressions to be bound from left to right, so that we can bind the whole expression to cause inference of its parts. The previous version of the code bound the right-hand-side before the left, which would cause an infinite recursion when trying to bind the enclosing expression to cause type inference to occur. #ByDesign

BoundExpression boundOp = BindSimpleBinaryOperator(syntaxNode, diagnostics, left, right, ref compoundStringLength);
expressions.Push(boundOp);
result = boundOp;
}

Debug.Assert(expressions.Count == 1);

var result = expressions.Peek();
expressions.Free();
syntaxNodes.Free();

return result;
}

Expand All @@ -440,8 +430,8 @@ private BoundExpression BindSimpleBinaryOperator(BinaryExpressionSyntax node, Di
{
BinaryOperatorKind kind = SyntaxKindToBinaryOperatorKind(node.Kind());

// If either operand is bad, don't try to do binary operator overload resolution; that will just
// make cascading errors.
// If either operand is bad, don't try to do binary operator overload resolution; that would just
// make cascading errors.

if (left.HasAnyErrors || right.HasAnyErrors)
{
Expand Down
8 changes: 2 additions & 6 deletions src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ private BoundPattern BindDeclarationPattern(
if (declType == (object)null)
{
Debug.Assert(hasErrors);
declType = this.CreateErrorType();
declType = this.CreateErrorType("var");
}

var boundDeclType = new BoundTypeExpression(typeSyntax, aliasOpt, inferredType: isVar, type: declType);
Expand Down Expand Up @@ -283,10 +283,7 @@ private BoundPattern BindDeclarationPattern(
LocalDeclarationKind.PatternVariable);
}

if (isVar)
{
localSymbol.SetTypeSymbol(operandType);
}
localSymbol.SetType(declType);

// Check for variable declaration errors.
hasErrors |= localSymbol.ScopeBinder.ValidateDeclarationNameConflictsInScope(localSymbol, diagnostics);
Expand All @@ -300,7 +297,6 @@ private BoundPattern BindDeclarationPattern(
hasErrors = true;
}

DeclareLocalVariable(localSymbol, identifier, declType);
return new BoundDeclarationPattern(node, localSymbol, boundDeclType, isVar, hasErrors);
}
}
Expand Down
41 changes: 10 additions & 31 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -912,12 +912,7 @@ protected BoundLocalDeclaration BindVariableDeclaration(
hasErrors = true;
}

Debug.Assert((object)localSymbol != null);

DeclareLocalVariable(
localSymbol,
declarator.Identifier,
declTypeOpt);
localSymbol.SetType(declTypeOpt);

if (localSymbol.RefKind != RefKind.None && initializerOpt != null)
{
Expand Down Expand Up @@ -972,13 +967,10 @@ private ImmutableArray<BoundExpression> BindDeclaratorArguments(VariableDeclarat

if (declarator.ArgumentList != null)
{
var builder = ArrayBuilder<BoundExpression>.GetInstance();
foreach (var argument in declarator.ArgumentList.Arguments)
{
var boundArgument = BindValue(argument.Expression, diagnostics, BindValueKind.RValue);
builder.Add(boundArgument);
}
arguments = builder.ToImmutableAndFree();
AnalyzedArguments analyzedArguments = AnalyzedArguments.GetInstance();
BindArgumentsAndNames(declarator.ArgumentList, diagnostics, analyzedArguments);
arguments = BuildArgumentsForErrorRecovery(analyzedArguments);
analyzedArguments.Free();
}

return arguments;
Expand Down Expand Up @@ -2290,24 +2282,6 @@ private BoundExpression BindPossibleArrayInitializer(
return BindUnexpectedArrayInitializer((InitializerExpressionSyntax)node, diagnostics, ErrorCode.ERR_ArrayInitToNonArrayType);
}

internal static void DeclareLocalVariable(
SourceLocalSymbol symbol,
SyntaxToken identifierToken,
TypeSymbol type)
{
// In the original compiler this
// method has many side effects; it sets the type
// of the local symbol, it gives errors if the local
// is a duplicate, it creates new symbols for lambda
// expressions, puts stuff in caches, and so on.

Debug.Assert((object)symbol != null);
Debug.Assert(symbol.IdentifierToken == identifierToken);
symbol.SetTypeSymbol(type);
// UNDONE: Can we come up with a way to set the type of a local which does
// UNDONE: not duplicate work and does not mutate the symbol?
}

protected virtual SourceLocalSymbol LookupLocal(SyntaxToken nameToken)
{
return Next.LookupLocal(nameToken);
Expand Down Expand Up @@ -2983,6 +2957,11 @@ internal virtual BoundStatement BindSwitchExpressionAndSections(SwitchStatementS
return this.Next.BindSwitchExpressionAndSections(node, originalBinder, diagnostics);
}

internal virtual void BindPatternSwitchLabelForInference(CasePatternSwitchLabelSyntax node, DiagnosticBag diagnostics)
{
this.Next.BindPatternSwitchLabelForInference(node, diagnostics);
}

private BoundStatement BindWhile(WhileStatementSyntax node, DiagnosticBag diagnostics)
{
Debug.Assert(node != null);
Expand Down
6 changes: 6 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,12 @@ internal override BoundStatement BindSwitchExpressionAndSections(SwitchStatement
throw ExceptionUtilities.Unreachable;
}

internal override void BindPatternSwitchLabelForInference(CasePatternSwitchLabelSyntax node, DiagnosticBag diagnostics)
{
// There's supposed to be a SwitchBinder (or other overrider of this method) in the chain.
throw ExceptionUtilities.Unreachable;
}

internal override BoundForStatement BindForParts(DiagnosticBag diagnostics, Binder originalBinder)
{
// There's supposed to be a ForLoopBinder (or other overrider of this method) in the chain.
Expand Down
Loading