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

Support long chains of else if statements #74317

Merged
merged 24 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
61 changes: 56 additions & 5 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2494,15 +2494,66 @@ private void GenerateImplicitConversionErrorsForTupleLiteralArguments(
}
}

#nullable enable
private BoundStatement BindIfStatement(IfStatementSyntax node, BindingDiagnosticBag diagnostics)
{
var condition = BindBooleanExpression(node.Condition, diagnostics);
var consequence = BindPossibleEmbeddedStatement(node.Statement, diagnostics);
BoundStatement alternative = (node.Else == null) ? null : BindPossibleEmbeddedStatement(node.Else.Statement, diagnostics);
return bindIfStatement(this, node, diagnostics);

BoundStatement result = new BoundIfStatement(node, condition, consequence, alternative);
return result;
static BoundStatement bindIfStatement(Binder binder, IfStatementSyntax node, BindingDiagnosticBag diagnostics)
{
var stack = ArrayBuilder<(Binder, IfStatementSyntax IfStatementSyntax, BoundExpression Condition, BoundStatement Consequence, bool WrapWithVariables)>.GetInstance();

BoundStatement? alternative;
bool wrapWithVariables = false;
while (true)
{
var condition = binder.BindBooleanExpression(node.Condition, diagnostics);
var consequence = binder.BindPossibleEmbeddedStatement(node.Statement, diagnostics);
stack.Push((binder, node, condition, consequence, wrapWithVariables));

if (node.Else == null)
{
alternative = null;
break;
}

var elseStatementSyntax = node.Else.Statement;
if (elseStatementSyntax is IfStatementSyntax ifStatementSyntax)
{
var b = binder.GetBinder(ifStatementSyntax);
Debug.Assert(b != null);
binder = b;
node = ifStatementSyntax;
wrapWithVariables = true;
}
else
{
alternative = binder.BindPossibleEmbeddedStatement(elseStatementSyntax, diagnostics);
break;
}
}

BoundStatement result;
do
{
BoundExpression condition;
BoundStatement consequence;
(binder, node, condition, consequence, wrapWithVariables) = stack.Pop();
result = new BoundIfStatement(node, condition, consequence, alternative);
if (wrapWithVariables)
{
result = binder.WrapWithVariablesIfAny(node, result);
}
alternative = result;
}
while (stack.Any());

stack.Free();

return result;
}
}
#nullable disable

internal BoundExpression BindBooleanExpression(ExpressionSyntax node, BindingDiagnosticBag diagnostics)
{
Expand Down
58 changes: 42 additions & 16 deletions src/Compilers/CSharp/Portable/Binder/LocalBinderFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -798,9 +798,29 @@ public override void VisitSwitchExpression(SwitchExpressionSyntax node)

public override void VisitIfStatement(IfStatementSyntax node)
{
Visit(node.Condition, _enclosing);
VisitPossibleEmbeddedStatement(node.Statement, _enclosing);
Visit(node.Else, _enclosing);
Binder enclosing = _enclosing;
while (true)
{
Visit(node.Condition, enclosing);
VisitPossibleEmbeddedStatement(node.Statement, enclosing);

if (node.Else == null)
{
break;
}

var elseStatementSyntax = node.Else.Statement;
if (elseStatementSyntax is IfStatementSyntax ifStatementSyntax)
{
node = ifStatementSyntax;
enclosing = GetBinderForPossibleEmbeddedStatement(node, enclosing);
}
else
{
VisitPossibleEmbeddedStatement(elseStatementSyntax, enclosing);
break;
}
}
}

public override void VisitElseClause(ElseClauseSyntax node)
Expand Down Expand Up @@ -1019,23 +1039,29 @@ private Binder GetBinderForPossibleEmbeddedStatement(StatementSyntax statement,
}
}

private void VisitPossibleEmbeddedStatement(StatementSyntax statement, Binder enclosing)
private Binder GetBinderForPossibleEmbeddedStatement(StatementSyntax statement, Binder enclosing)
{
if (statement != null)
CSharpSyntaxNode embeddedScopeDesignator;
// Some statements by default do not introduce its own scope for locals.
// For example: Expression Statement, Return Statement, etc. However,
// when a statement like that is an embedded statement (like IfStatementSyntax.Statement),
// then it should introduce a scope for locals declared within it. Here we are detecting
// such statements and creating a binder that should own the scope.
enclosing = GetBinderForPossibleEmbeddedStatement(statement, enclosing, out embeddedScopeDesignator);

if (embeddedScopeDesignator != null)
{
CSharpSyntaxNode embeddedScopeDesignator;
// Some statements by default do not introduce its own scope for locals.
// For example: Expression Statement, Return Statement, etc. However,
// when a statement like that is an embedded statement (like IfStatementSyntax.Statement),
// then it should introduce a scope for locals declared within it. Here we are detecting
// such statements and creating a binder that should own the scope.
enclosing = GetBinderForPossibleEmbeddedStatement(statement, enclosing, out embeddedScopeDesignator);
AddToMap(embeddedScopeDesignator, enclosing);
}

if (embeddedScopeDesignator != null)
{
AddToMap(embeddedScopeDesignator, enclosing);
}
return enclosing;
}

private void VisitPossibleEmbeddedStatement(StatementSyntax statement, Binder enclosing)
{
if (statement != null)
{
enclosing = GetBinderForPossibleEmbeddedStatement(statement, enclosing);
Visit(statement, enclosing);
}
}
Expand Down
27 changes: 27 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/RefSafetyAnalysis.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1112,6 +1112,33 @@ private static ImmutableArray<BoundExpression> GetDeconstructionRightParts(Bound
return null;
}

public override BoundNode? VisitIfStatement(BoundIfStatement node)
{
while (true)
{
this.Visit(node.Condition);
this.Visit(node.Consequence);

var alternative = node.AlternativeOpt;
if (alternative is null)
{
break;
}

if (alternative is BoundIfStatement elseIfStatement)
{
node = elseIfStatement;
}
else
{
this.Visit(alternative);
break;
}
}

return null;
}

private static void Error(BindingDiagnosticBag diagnostics, ErrorCode code, SyntaxNodeOrToken syntax, params object[] args)
{
var location = syntax.GetLocation();
Expand Down
24 changes: 24 additions & 0 deletions src/Compilers/CSharp/Portable/BoundTree/BoundNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,30 @@ private void CheckDeclared(LocalSymbol local)
return null;
}

public override BoundNode? VisitIfStatement(BoundIfStatement node)
Copy link
Contributor

@AlekseyTs AlekseyTs Sep 19, 2024

Choose a reason for hiding this comment

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

public override BoundNode? VisitIfStatement(BoundIfStatement node)

Move this to BoundTreeWalkerWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperator? #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good.

{
while (true)
{
Visit(node.Condition);
Visit(node.Consequence);
var alternative = node.AlternativeOpt;
if (alternative is null)
{
break;
}
if (alternative is BoundIfStatement elseIfStatement)
{
node = elseIfStatement;
}
else
{
Visit(alternative);
break;
}
}
return null;
}

public override BoundNode? VisitFixedStatement(BoundFixedStatement node)
{
AddAll(node.Locals);
Expand Down
43 changes: 43 additions & 0 deletions src/Compilers/CSharp/Portable/BoundTree/NullabilityRewriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,49 @@ internal sealed partial class NullabilityRewriter : BoundTreeRewriter
return VisitBinaryOperatorBase(node);
}

public override BoundNode? VisitIfStatement(BoundIfStatement node)
{
var stack = ArrayBuilder<(BoundIfStatement, BoundExpression, BoundStatement)>.GetInstance();

BoundStatement? rewrittenAlternative;
while (true)
{
var rewrittenCondition = (BoundExpression)Visit(node.Condition);
var rewrittenConsequence = (BoundStatement)Visit(node.Consequence);
Debug.Assert(rewrittenConsequence is { });
stack.Push((node, rewrittenCondition, rewrittenConsequence));

var alternative = node.AlternativeOpt;
if (alternative is null)
{
rewrittenAlternative = null;
break;
}

if (alternative is BoundIfStatement elseIfStatement)
{
node = elseIfStatement;
}
else
{
rewrittenAlternative = (BoundStatement)Visit(alternative);
break;
}
}

BoundStatement result;
do
{
var (ifStatement, rewrittenCondition, rewrittenConsequence) = stack.Pop();
result = ifStatement.Update(rewrittenCondition, rewrittenConsequence, rewrittenAlternative);
rewrittenAlternative = result;
}
while (stack.Any());

stack.Free();
return result;
}

private BoundNode VisitBinaryOperatorBase(BoundBinaryOperatorBase binaryOperator)
{
// Use an explicit stack to avoid blowing the managed stack when visiting deeply-recursive
Expand Down
29 changes: 20 additions & 9 deletions src/Compilers/CSharp/Portable/CodeGen/CodeGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,9 @@ public CodeGenerator(
debugFriendly: _ilEmitStyle != ILEmitStyle.Release,
stackLocals: out _stackLocals);
}
catch (BoundTreeVisitor.CancelledByStackGuardException ex)
catch (BoundTreeVisitor.CancelledByStackGuardException)
{
ex.AddAnError(diagnostics);
//ex.AddAnError(diagnostics); // PROTOTYPE: Revert the changes to CodeGenerator and Optimizer and instead change VisitStatementList().
_boundBody = boundBody;
}

Expand Down Expand Up @@ -393,13 +393,29 @@ private void EmitSequencePointStatement(BoundSequencePoint node)
}
}

private void EmitSequencePointStatement(BoundSequencePointWithSpan node)
private void EmitSequencePointStatementBegin(BoundSequencePointWithSpan node)
{
TextSpan span = node.Span;
if (span != default(TextSpan) && _emitPdbSequencePoints)
{
this.EmitSequencePoint(node.SyntaxTree, span);
}
}

private void EmitSequencePointStatementEnd(BoundSequencePointWithSpan node, int instructionsEmitted)
{
TextSpan span = node.Span;
if (instructionsEmitted == 0 && span != default(TextSpan) && _ilEmitStyle == ILEmitStyle.Debug)
{
// if there was no code emitted, then emit nop
// otherwise this point could get associated with some random statement, possibly in a wrong scope
_builder.EmitOpCode(ILOpCode.Nop);
}
}

private void EmitSequencePointStatement(BoundSequencePointWithSpan node)
{
EmitSequencePointStatementBegin(node);

BoundStatement statement = node.StatementOpt;
int instructionsEmitted = 0;
Expand All @@ -408,12 +424,7 @@ private void EmitSequencePointStatement(BoundSequencePointWithSpan node)
instructionsEmitted = this.EmitStatementAndCountInstructions(statement);
}

if (instructionsEmitted == 0 && span != default(TextSpan) && _ilEmitStyle == ILEmitStyle.Debug)
{
// if there was no code emitted, then emit nop
// otherwise this point could get associated with some random statement, possibly in a wrong scope
_builder.EmitOpCode(ILOpCode.Nop);
}
EmitSequencePointStatementEnd(node, instructionsEmitted);
}

private void EmitSavePreviousSequencePoint(BoundSavePreviousSequencePoint statement)
Expand Down
30 changes: 28 additions & 2 deletions src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,36 @@ private int EmitStatementAndCountInstructions(BoundStatement statement)

private void EmitStatementList(BoundStatementList list)
{
for (int i = 0, n = list.Statements.Length; i < n; i++)
var stack = ArrayBuilder<(BoundStatementList, int, BoundSequencePointWithSpan, int)>.GetInstance();

stack.Push((list, 0, null, 0));

while (stack.Any())
{
EmitStatement(list.Statements[i]);
int i;
BoundSequencePointWithSpan sequencePointWithSpan;
int instructionsEmittedBegin;
(list, i, sequencePointWithSpan, instructionsEmittedBegin) = stack.Pop();
for (int n = list.Statements.Length; i < n; i++)
{
var statement = list.Statements[i];
if (statement is BoundSequencePointWithSpan { StatementOpt: BoundStatementList { Kind: BoundKind.StatementList } nestedList } nestedSequencePointWithSpan)
{
EmitSequencePointStatementBegin(nestedSequencePointWithSpan);
stack.Push((list, i + 1, sequencePointWithSpan, instructionsEmittedBegin));
stack.Push((nestedList, 0, nestedSequencePointWithSpan, _builder.InstructionsEmitted));
goto endOfLoop;
}
EmitStatement(statement);
}
if (sequencePointWithSpan is { })
{
EmitSequencePointStatementEnd(sequencePointWithSpan, _builder.InstructionsEmitted - instructionsEmittedBegin);
}
endOfLoop: continue;
}

stack.Free();
}

private void EmitNoOpStatement(BoundNoOpStatement statement)
Expand Down
25 changes: 24 additions & 1 deletion src/Compilers/CSharp/Portable/CodeGen/Optimizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,30 @@ private void PopEvalStack()
public BoundNode VisitStatement(BoundNode node)
{
Debug.Assert(node == null || EvalStackIsEmpty());
return VisitSideEffect(node);

_recursionDepth++;

BoundNode result;
if (_recursionDepth > 1)
{
StackGuard.EnsureSufficientExecutionStack(_recursionDepth);

result = VisitSideEffect(node);
}
else
{
try
{
result = VisitSideEffect(node);
}
catch (InsufficientExecutionStackException ex)
{
throw new CancelledByStackGuardException(ex, node);
}
}

_recursionDepth--;
return result;
}

public BoundNode VisitSideEffect(BoundNode node)
Expand Down
Loading
Loading