diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index 607f7b90e2f1b..15cba6704b579 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -2494,15 +2494,64 @@ 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)>.GetInstance(); + + BoundStatement? alternative; + while (true) + { + var condition = binder.BindBooleanExpression(node.Condition, diagnostics); + var consequence = binder.BindPossibleEmbeddedStatement(node.Statement, diagnostics); + stack.Push((binder, node, condition, consequence)); + + 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; + } + else + { + alternative = binder.BindPossibleEmbeddedStatement(elseStatementSyntax, diagnostics); + break; + } + } + + BoundStatement result; + do + { + BoundExpression condition; + BoundStatement consequence; + (binder, node, condition, consequence) = stack.Pop(); + result = new BoundIfStatement(node, condition, consequence, alternative); + if (stack.Any()) + { + result = binder.WrapWithVariablesIfAny(node, result); + } + alternative = result; + } + while (stack.Any()); + + stack.Free(); + + return result; + } } +#nullable disable internal BoundExpression BindBooleanExpression(ExpressionSyntax node, BindingDiagnosticBag diagnostics) { diff --git a/src/Compilers/CSharp/Portable/Binder/LocalBinderFactory.cs b/src/Compilers/CSharp/Portable/Binder/LocalBinderFactory.cs index 1e6ee6bfc4953..9a2e9e9ae4234 100644 --- a/src/Compilers/CSharp/Portable/Binder/LocalBinderFactory.cs +++ b/src/Compilers/CSharp/Portable/Binder/LocalBinderFactory.cs @@ -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) @@ -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); } } diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundTreeRewriter.cs b/src/Compilers/CSharp/Portable/BoundTree/BoundTreeRewriter.cs index 6f489cb0cf2ae..02099bcd64cc7 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundTreeRewriter.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundTreeRewriter.cs @@ -152,5 +152,48 @@ protected BoundTreeRewriterWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperat return left; } + + public sealed override BoundNode? VisitIfStatement(BoundIfStatement node) + { + if (node.AlternativeOpt is not BoundIfStatement ifStatement) + { + return base.VisitIfStatement(node); + } + + var stack = ArrayBuilder.GetInstance(); + stack.Push(node); + + BoundStatement? alternative; + while (true) + { + stack.Push(ifStatement); + + alternative = ifStatement.AlternativeOpt; + if (alternative is not BoundIfStatement nextIfStatement) + { + break; + } + + ifStatement = nextIfStatement; + } + + alternative = (BoundStatement?)this.Visit(alternative); + + do + { + ifStatement = stack.Pop(); + + BoundExpression condition = (BoundExpression)this.Visit(ifStatement.Condition); + BoundStatement consequence = (BoundStatement)this.Visit(ifStatement.Consequence); + + alternative = ifStatement.Update(condition, consequence, alternative); + } + while (stack.Count > 0); + + Debug.Assert((object)ifStatement == node); + stack.Free(); + + return alternative; + } } } diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundTreeWalker.cs b/src/Compilers/CSharp/Portable/BoundTree/BoundTreeWalker.cs index 8f0df23b1e56c..030b62c8d0d02 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundTreeWalker.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundTreeWalker.cs @@ -195,5 +195,29 @@ protected virtual void VisitArguments(BoundCall node) { this.VisitList(node.Arguments); } + + public sealed override BoundNode? VisitIfStatement(BoundIfStatement node) + { + 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; + } } } diff --git a/src/Compilers/CSharp/Portable/BoundTree/NullabilityRewriter.cs b/src/Compilers/CSharp/Portable/BoundTree/NullabilityRewriter.cs index db5389f0b60c1..a579f94cb1262 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/NullabilityRewriter.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/NullabilityRewriter.cs @@ -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 diff --git a/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.NodeMapBuilder.cs b/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.NodeMapBuilder.cs index 1b5ecb4b89077..5c4b3fcc59247 100644 --- a/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.NodeMapBuilder.cs +++ b/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.NodeMapBuilder.cs @@ -5,14 +5,12 @@ #nullable disable using System.Collections.Generic; -using System.Collections.Immutable; +using System.Diagnostics; using Microsoft.CodeAnalysis.Collections; +using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.PooledObjects; using Roslyn.Utilities; -using System.Diagnostics; -using System.Linq; -using Microsoft.CodeAnalysis.CSharp.Symbols; namespace Microsoft.CodeAnalysis.CSharp { @@ -287,6 +285,37 @@ public override BoundNode VisitBinaryOperator(BoundBinaryOperator node) throw ExceptionUtilities.Unreachable(); } + 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; + if (ShouldAddNode(node)) + { + _map.Add(node.Syntax, node); + } + } + else + { + this.Visit(alternative); + break; + } + } + + return null; + } + protected override bool ConvertInsufficientExecutionStackExceptionToCancelledByStackGuardException() { return false; diff --git a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs index b67875f9192bb..303c221951270 100644 --- a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs +++ b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs @@ -2180,7 +2180,7 @@ private static bool IsEmptyRewritePossible(BoundNode node) } } - private sealed class EmptyRewriter : BoundTreeRewriterWithStackGuard + private sealed class EmptyRewriter : BoundTreeRewriterWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperator { } diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs index 9d4bef8a46fe0..dbe7ac50e5b22 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs @@ -1704,22 +1704,54 @@ protected virtual void AfterVisitConversion(BoundConversion node) { } - public override BoundNode VisitIfStatement(BoundIfStatement node) + public sealed override BoundNode VisitIfStatement(BoundIfStatement node) { // 5.3.3.5 If statements - VisitCondition(node.Condition); - TLocalState trueState = StateWhenTrue; - TLocalState falseState = StateWhenFalse; - SetState(trueState); - VisitStatement(node.Consequence); - trueState = this.State; - SetState(falseState); - if (node.AlternativeOpt != null) + + var stack = ArrayBuilder<(TLocalState, BoundIfStatement)>.GetInstance(); + + TLocalState trueState; + while (true) { - VisitStatement(node.AlternativeOpt); + VisitCondition(node.Condition); + trueState = StateWhenTrue; + TLocalState falseState = StateWhenFalse; + SetState(trueState); + VisitStatement(node.Consequence); + trueState = this.State; + SetState(falseState); + + var alternative = node.AlternativeOpt; + if (alternative is null) + { + break; + } + + if (alternative is BoundIfStatement elseIfStatement) + { + node = elseIfStatement; + stack.Push((trueState, node)); + EnterRegionIfNeeded(node); + } + else + { + VisitStatement(alternative); + break; + } + } + + while (true) + { + Join(ref this.State, ref trueState); + if (!stack.Any()) + { + break; + } + (trueState, node) = stack.Pop(); + LeaveRegionIfNeeded(node); } - Join(ref this.State, ref trueState); + stack.Free(); return null; } diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.DebugVerifier.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.DebugVerifier.cs index 9fad0aeeb0729..04757fa99dafe 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.DebugVerifier.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.DebugVerifier.cs @@ -159,6 +159,33 @@ private void VerifyExpression(BoundExpression expression, bool overrideSkippedEx 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; + } + public override BoundNode? VisitForEachStatement(BoundForEachStatement node) { Visit(node.IterationVariableType); diff --git a/src/Compilers/CSharp/Portable/Lowering/DiagnosticsPass_ExpressionTrees.cs b/src/Compilers/CSharp/Portable/Lowering/DiagnosticsPass_ExpressionTrees.cs index 1f401b2ef6eff..4e08050b2124e 100644 --- a/src/Compilers/CSharp/Portable/Lowering/DiagnosticsPass_ExpressionTrees.cs +++ b/src/Compilers/CSharp/Portable/Lowering/DiagnosticsPass_ExpressionTrees.cs @@ -1045,5 +1045,32 @@ public override BoundNode VisitCollectionExpression(BoundCollectionExpression no return base.VisitCollectionExpression(node); } + + 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; + } } } diff --git a/src/Compilers/CSharp/Portable/Lowering/Instrumentation/CodeCoverageInstrumenter.cs b/src/Compilers/CSharp/Portable/Lowering/Instrumentation/CodeCoverageInstrumenter.cs index 7f0e79edfd813..5c57cc091dab8 100644 --- a/src/Compilers/CSharp/Portable/Lowering/Instrumentation/CodeCoverageInstrumenter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/Instrumentation/CodeCoverageInstrumenter.cs @@ -371,9 +371,9 @@ public override BoundStatement InstrumentForEachStatementDeconstructionVariables return AddDynamicAnalysis(original, base.InstrumentForEachStatementDeconstructionVariablesDeclaration(original, iterationVarDecl)); } - public override BoundStatement InstrumentIfStatement(BoundIfStatement original, BoundStatement rewritten) + public override BoundStatement InstrumentIfStatementConditionalGoto(BoundIfStatement original, BoundStatement rewritten) { - return AddDynamicAnalysis(original, base.InstrumentIfStatement(original, rewritten)); + return AddDynamicAnalysis(original, base.InstrumentIfStatementConditionalGoto(original, rewritten)); } public override BoundStatement InstrumentWhileStatementConditionalGotoStartOrBreak(BoundWhileStatement original, BoundStatement ifConditionGotoStart) diff --git a/src/Compilers/CSharp/Portable/Lowering/Instrumentation/CompoundInstrumenter.cs b/src/Compilers/CSharp/Portable/Lowering/Instrumentation/CompoundInstrumenter.cs index 82f5dda4cb4b9..289614c47adf9 100644 --- a/src/Compilers/CSharp/Portable/Lowering/Instrumentation/CompoundInstrumenter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/Instrumentation/CompoundInstrumenter.cs @@ -131,9 +131,9 @@ public override BoundExpression InstrumentForStatementCondition(BoundForStatemen return Previous.InstrumentForStatementCondition(original, rewrittenCondition, factory); } - public override BoundStatement InstrumentIfStatement(BoundIfStatement original, BoundStatement rewritten) + public override BoundStatement InstrumentIfStatementConditionalGoto(BoundIfStatement original, BoundStatement rewritten) { - return Previous.InstrumentIfStatement(original, rewritten); + return Previous.InstrumentIfStatementConditionalGoto(original, rewritten); } public override BoundExpression InstrumentIfStatementCondition(BoundIfStatement original, BoundExpression rewrittenCondition, SyntheticBoundNodeFactory factory) diff --git a/src/Compilers/CSharp/Portable/Lowering/Instrumentation/DebugInfoInjector.cs b/src/Compilers/CSharp/Portable/Lowering/Instrumentation/DebugInfoInjector.cs index 5f12229415f08..a9fabf63b6b51 100644 --- a/src/Compilers/CSharp/Portable/Lowering/Instrumentation/DebugInfoInjector.cs +++ b/src/Compilers/CSharp/Portable/Lowering/Instrumentation/DebugInfoInjector.cs @@ -355,12 +355,12 @@ public override BoundExpression InstrumentForStatementCondition(BoundForStatemen return AddConditionSequencePoint(base.InstrumentForStatementCondition(original, rewrittenCondition, factory), original.Syntax, factory); } - public override BoundStatement InstrumentIfStatement(BoundIfStatement original, BoundStatement rewritten) + public override BoundStatement InstrumentIfStatementConditionalGoto(BoundIfStatement original, BoundStatement rewritten) { var syntax = (IfStatementSyntax)original.Syntax; return new BoundSequencePointWithSpan( syntax, - base.InstrumentIfStatement(original, rewritten), + base.InstrumentIfStatementConditionalGoto(original, rewritten), TextSpan.FromBounds( syntax.IfKeyword.SpanStart, syntax.CloseParenToken.Span.End), diff --git a/src/Compilers/CSharp/Portable/Lowering/Instrumentation/Instrumenter.cs b/src/Compilers/CSharp/Portable/Lowering/Instrumentation/Instrumenter.cs index 561559771f6c2..0114fd9055ac3 100644 --- a/src/Compilers/CSharp/Portable/Lowering/Instrumentation/Instrumenter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/Instrumentation/Instrumenter.cs @@ -189,7 +189,7 @@ public virtual BoundExpression InstrumentForStatementCondition(BoundForStatement return rewrittenCondition; } - public virtual BoundStatement InstrumentIfStatement(BoundIfStatement original, BoundStatement rewritten) + public virtual BoundStatement InstrumentIfStatementConditionalGoto(BoundIfStatement original, BoundStatement rewritten) { Debug.Assert(original.Syntax.Kind() == SyntaxKind.IfStatement); return InstrumentStatement(original, rewritten); diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs index 59cd4fd4afe8a..0dd4d8a7742db 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs @@ -1143,6 +1143,17 @@ private CompoundUseSiteInfo GetNewCompoundUseSiteInfo() /// private sealed class LocalRewritingValidator : BoundTreeWalkerWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperator { + public override BoundNode? Visit(BoundNode? node) + { + if (node is BoundIfStatement) + { + Fail(node); + return null; + } + + return base.Visit(node); + } + /// /// Asserts that no unexpected nodes survived local rewriting. /// @@ -1170,12 +1181,6 @@ public static void Validate(BoundNode node) return null; } - public override BoundNode? VisitIfStatement(BoundIfStatement node) - { - Fail(node); - return null; - } - public override BoundNode? VisitDeconstructionVariablePendingInference(DeconstructionVariablePendingInference node) { Fail(node); diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ForEachStatement.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ForEachStatement.cs index 7937be4379a86..306b42bad04f0 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ForEachStatement.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ForEachStatement.cs @@ -404,7 +404,6 @@ private bool TryGetDisposeMethod(SyntaxNode forEachSyntax, ForEachEnumeratorInfo syntax: forEachSyntax, rewrittenCondition: _factory.IsNotNullReference(boundEnumeratorVar), rewrittenConsequence: disposeCallStatement, - rewrittenAlternativeOpt: null, hasErrors: false); } @@ -459,7 +458,6 @@ private bool TryGetDisposeMethod(SyntaxNode forEachSyntax, ForEachEnumeratorInfo resultKind: LookupResultKind.Viable, type: _compilation.GetSpecialType(SpecialType.System_Boolean)), rewrittenConsequence: disposeCallStatement, - rewrittenAlternativeOpt: null, hasErrors: false); // IDisposable d = e as IDisposable; diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_IfStatement.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_IfStatement.cs index e4f8d4642097b..6bdd62e1dfa93 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_IfStatement.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_IfStatement.cs @@ -6,8 +6,6 @@ using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.PooledObjects; -using Microsoft.CodeAnalysis.Text; -using System.Collections.Immutable; namespace Microsoft.CodeAnalysis.CSharp { @@ -16,87 +14,132 @@ internal sealed partial class LocalRewriter public override BoundNode VisitIfStatement(BoundIfStatement node) { Debug.Assert(node != null); - var rewrittenCondition = VisitExpression(node.Condition); - var rewrittenConsequence = VisitStatement(node.Consequence); - Debug.Assert(rewrittenConsequence is { }); - var rewrittenAlternative = VisitStatement(node.AlternativeOpt); - var syntax = (IfStatementSyntax)node.Syntax; - - // EnC: We need to insert a hidden sequence point to handle function remapping in case - // the containing method is edited while methods invoked in the condition are being executed. - if (this.Instrument && !node.WasCompilerGenerated) + + var stack = ArrayBuilder<(BoundIfStatement, GeneratedLabelSymbol, int)>.GetInstance(); + var builder = ArrayBuilder.GetInstance(); + + while (true) { - rewrittenCondition = Instrumenter.InstrumentIfStatementCondition(node, rewrittenCondition, _factory); - } + var rewrittenCondition = VisitExpression(node.Condition); + var rewrittenConsequence = VisitStatement(node.Consequence); + Debug.Assert(rewrittenConsequence is { }); + + // EnC: We need to insert a hidden sequence point to handle function remapping in case + // the containing method is edited while methods invoked in the condition are being executed. + if (this.Instrument && !node.WasCompilerGenerated) + { + rewrittenCondition = Instrumenter.InstrumentIfStatementCondition(node, rewrittenCondition, _factory); + } + + var elseIfStatement = node.AlternativeOpt as BoundIfStatement; + BoundStatement? rewrittenAlternative = null; + + if (elseIfStatement is null) + { + rewrittenAlternative = VisitStatement(node.AlternativeOpt); + } + + var afterif = new GeneratedLabelSymbol("afterif"); + stack.Push((node, afterif, builder.Count)); - var result = RewriteIfStatement(syntax, rewrittenCondition, rewrittenConsequence, rewrittenAlternative, node.HasErrors); + if (elseIfStatement is null && rewrittenAlternative is null) + { + // if (condition) + // consequence; + // + // becomes + // + // GotoIfFalse condition afterif; + // consequence; + // afterif: + + builder.Add(new BoundConditionalGoto(rewrittenCondition.Syntax, rewrittenCondition, false, afterif)); + builder.Add(rewrittenConsequence); + break; + } + else + { + // if (condition) + // consequence; + // else + // alternative; + // + // becomes + // + // GotoIfFalse condition alt; + // consequence + // goto afterif; + // alt: + // alternative; + // afterif: + + var alt = new GeneratedLabelSymbol("alternative"); + + builder.Add(new BoundConditionalGoto(rewrittenCondition.Syntax, rewrittenCondition, false, alt)); + builder.Add(rewrittenConsequence); + builder.Add(BoundSequencePoint.CreateHidden()); + var syntax = (IfStatementSyntax)node.Syntax; + builder.Add(new BoundGotoStatement(syntax, afterif)); + builder.Add(new BoundLabelStatement(syntax, alt)); + + if (rewrittenAlternative is not null) + { + builder.Add(rewrittenAlternative); + break; + } + + Debug.Assert(elseIfStatement is not null); + + node = elseIfStatement; + } + } - // add sequence point before the whole statement - if (this.Instrument && !node.WasCompilerGenerated) + do { - result = Instrumenter.InstrumentIfStatement(node, result); + (node, var afterif, var conditionalGotoIndex) = stack.Pop(); + Debug.Assert(builder[conditionalGotoIndex] is BoundConditionalGoto); + + var syntax = (IfStatementSyntax)node.Syntax; + + builder.Add(BoundSequencePoint.CreateHidden()); + builder.Add(new BoundLabelStatement(syntax, afterif)); + + // add sequence point before the whole statement + if (this.Instrument && !node.WasCompilerGenerated) + { + builder[conditionalGotoIndex] = Instrumenter.InstrumentIfStatementConditionalGoto(node, builder[conditionalGotoIndex]); + } } + while (stack.Any()); - return result; + stack.Free(); + return new BoundStatementList(node.Syntax, builder.ToImmutableAndFree(), node.HasErrors); } private static BoundStatement RewriteIfStatement( SyntaxNode syntax, BoundExpression rewrittenCondition, BoundStatement rewrittenConsequence, - BoundStatement? rewrittenAlternativeOpt, bool hasErrors) { var afterif = new GeneratedLabelSymbol("afterif"); var builder = ArrayBuilder.GetInstance(); - if (rewrittenAlternativeOpt == null) - { - // if (condition) - // consequence; - // - // becomes - // - // GotoIfFalse condition afterif; - // consequence; - // afterif: - - builder.Add(new BoundConditionalGoto(rewrittenCondition.Syntax, rewrittenCondition, false, afterif)); - builder.Add(rewrittenConsequence); - builder.Add(BoundSequencePoint.CreateHidden()); - builder.Add(new BoundLabelStatement(syntax, afterif)); - var statements = builder.ToImmutableAndFree(); - return new BoundStatementList(syntax, statements, hasErrors); - } - else - { - // if (condition) - // consequence; - // else - // alternative - // - // becomes - // - // GotoIfFalse condition alt; - // consequence - // goto afterif; - // alt: - // alternative; - // afterif: - - var alt = new GeneratedLabelSymbol("alternative"); - - builder.Add(new BoundConditionalGoto(rewrittenCondition.Syntax, rewrittenCondition, false, alt)); - builder.Add(rewrittenConsequence); - builder.Add(BoundSequencePoint.CreateHidden()); - builder.Add(new BoundGotoStatement(syntax, afterif)); - builder.Add(new BoundLabelStatement(syntax, alt)); - builder.Add(rewrittenAlternativeOpt); - builder.Add(BoundSequencePoint.CreateHidden()); - builder.Add(new BoundLabelStatement(syntax, afterif)); - return new BoundStatementList(syntax, builder.ToImmutableAndFree(), hasErrors); - } + // if (condition) + // consequence; + // + // becomes + // + // GotoIfFalse condition afterif; + // consequence; + // afterif: + builder.Add(new BoundConditionalGoto(rewrittenCondition.Syntax, rewrittenCondition, false, afterif)); + builder.Add(rewrittenConsequence); + builder.Add(BoundSequencePoint.CreateHidden()); + builder.Add(new BoundLabelStatement(syntax, afterif)); + var statements = builder.ToImmutableAndFree(); + return new BoundStatementList(syntax, statements, hasErrors); } } } diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_LockStatement.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_LockStatement.cs index 70692a4d9b647..7df64ae2978e2 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_LockStatement.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_LockStatement.cs @@ -163,7 +163,6 @@ public override BoundNode VisitLockStatement(BoundLockStatement node) lockSyntax, boundLockTakenTemp, exitCall, - null, node.HasErrors); return new BoundBlock( diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_UsingStatement.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_UsingStatement.cs index 249c7b23c0f1e..a7dcc68c4d583 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_UsingStatement.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_UsingStatement.cs @@ -410,7 +410,6 @@ private BoundStatement RewriteUsingStatementTryFinally( syntax: resourceSyntax, rewrittenCondition: ifCondition, rewrittenConsequence: disposeStatement, - rewrittenAlternativeOpt: null, hasErrors: false); } diff --git a/src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs b/src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs index 2b7e62c5393bc..bfe4dcad1c618 100644 --- a/src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs +++ b/src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs @@ -1801,15 +1801,44 @@ private IEmptyOperation CreateBoundNoOpStatementOperation(BoundNoOpStatement bou private IConditionalOperation CreateBoundIfStatementOperation(BoundIfStatement boundIfStatement) { - IOperation condition = Create(boundIfStatement.Condition); - IOperation whenTrue = Create(boundIfStatement.Consequence); - IOperation? whenFalse = Create(boundIfStatement.AlternativeOpt); - bool isRef = false; - SyntaxNode syntax = boundIfStatement.Syntax; - ITypeSymbol? type = null; - ConstantValue? constantValue = null; - bool isImplicit = boundIfStatement.WasCompilerGenerated; - return new ConditionalOperation(condition, whenTrue, whenFalse, isRef, _semanticModel, syntax, type, constantValue, isImplicit); + var stack = ArrayBuilder.GetInstance(); + + IOperation? whenFalse; + while (true) + { + stack.Push(boundIfStatement); + + var alternative = boundIfStatement.AlternativeOpt; + + if (alternative is BoundIfStatement elseIfStatement) + { + boundIfStatement = elseIfStatement; + } + else + { + whenFalse = Create(alternative); + break; + } + } + + ConditionalOperation result; + do + { + boundIfStatement = stack.Pop(); + IOperation condition = Create(boundIfStatement.Condition); + IOperation whenTrue = Create(boundIfStatement.Consequence); + bool isRef = false; + SyntaxNode syntax = boundIfStatement.Syntax; + ITypeSymbol? type = null; + ConstantValue? constantValue = null; + bool isImplicit = boundIfStatement.WasCompilerGenerated; + result = new ConditionalOperation(condition, whenTrue, whenFalse, isRef, _semanticModel, syntax, type, constantValue, isImplicit); + whenFalse = result; + } + while (stack.Any()); + + stack.Free(); + return result; } private IWhileLoopOperation CreateBoundWhileStatementOperation(BoundWhileStatement boundWhileStatement) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index d68834aefdfe3..427b8d72c71b1 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -7938,9 +7938,9 @@ or SyntaxKind.MinusMinusToken /// Those will instead be parsed out as script-fields/methods. private StatementSyntax ParseStatementCore(SyntaxList attributes, bool isGlobal) { - if (canReuseStatement(attributes, isGlobal)) + if (TryReuseStatement(attributes, isGlobal) is { } reused) { - return (StatementSyntax)this.EatNode(); + return reused; } ResetPoint resetPointBeforeStatement = this.GetResetPoint(); @@ -8016,14 +8016,19 @@ private StatementSyntax ParseStatementCore(SyntaxList attri _recursionDepth--; this.Release(ref resetPointBeforeStatement); } + } - bool canReuseStatement(SyntaxList attributes, bool isGlobal) + private StatementSyntax TryReuseStatement(SyntaxList attributes, bool isGlobal) + { + if (this.IsIncrementalAndFactoryContextMatches && + this.CurrentNode is Syntax.StatementSyntax && + !isGlobal && // Top-level statements are reused by ParseMemberDeclarationOrStatementCore when possible. + attributes.Count == 0) { - return this.IsIncrementalAndFactoryContextMatches && - this.CurrentNode is Syntax.StatementSyntax && - !isGlobal && // Top-level statements are reused by ParseMemberDeclarationOrStatementCore when possible. - attributes.Count == 0; + return (StatementSyntax)this.EatNode(); } + + return null; } private StatementSyntax ParseStatementCoreRest(SyntaxList attributes, bool isGlobal, ref ResetPoint resetPointBeforeStatement) @@ -9537,14 +9542,65 @@ private IfStatementSyntax ParseIfStatement(SyntaxList attri { Debug.Assert(this.CurrentToken.Kind == SyntaxKind.IfKeyword); - return _syntaxFactory.IfStatement( - attributes, - this.EatToken(SyntaxKind.IfKeyword), - this.EatToken(SyntaxKind.OpenParenToken), - this.ParseExpressionCore(), - this.EatToken(SyntaxKind.CloseParenToken), - this.ParseEmbeddedStatement(), - this.ParseElseClauseOpt()); + var stack = ArrayBuilder<(SyntaxToken, SyntaxToken, ExpressionSyntax, SyntaxToken, StatementSyntax, SyntaxToken)>.GetInstance(); + + StatementSyntax alternative = null; + while (true) + { + var ifKeyword = this.EatToken(SyntaxKind.IfKeyword); + var openParen = this.EatToken(SyntaxKind.OpenParenToken); + var condition = this.ParseExpressionCore(); + var closeParen = this.EatToken(SyntaxKind.CloseParenToken); + var consequence = this.ParseEmbeddedStatement(); + + var elseKeyword = this.CurrentToken.Kind != SyntaxKind.ElseKeyword ? + null : + this.EatToken(SyntaxKind.ElseKeyword); + stack.Push((ifKeyword, openParen, condition, closeParen, consequence, elseKeyword)); + + if (elseKeyword is null) + { + alternative = null; + break; + } + + if (this.CurrentToken.Kind != SyntaxKind.IfKeyword) + { + alternative = this.ParseEmbeddedStatement(); + break; + } + + alternative = TryReuseStatement(attributes: default, isGlobal: false); + if (alternative is not null) + { + break; + } + } + + IfStatementSyntax ifStatement; + do + { + var (ifKeyword, openParen, condition, closeParen, consequence, elseKeyword) = stack.Pop(); + var elseClause = alternative is null ? + null : + _syntaxFactory.ElseClause( + elseKeyword, + alternative); + ifStatement = _syntaxFactory.IfStatement( + attributeLists: stack.Any() ? default : attributes, + ifKeyword, + openParen, + condition, + closeParen, + consequence, + elseClause); + alternative = ifStatement; + } + while (stack.Any()); + + stack.Free(); + + return ifStatement; } private IfStatementSyntax ParseMisplacedElse(SyntaxList attributes) diff --git a/src/Compilers/CSharp/Portable/Syntax/LookupPosition.cs b/src/Compilers/CSharp/Portable/Syntax/LookupPosition.cs index 9015176b4b586..04d606e0d09c5 100644 --- a/src/Compilers/CSharp/Portable/Syntax/LookupPosition.cs +++ b/src/Compilers/CSharp/Portable/Syntax/LookupPosition.cs @@ -403,9 +403,7 @@ internal static SyntaxToken GetFirstExcludedToken(StatementSyntax statement) case SyntaxKind.GotoStatement: return ((GotoStatementSyntax)statement).SemicolonToken; case SyntaxKind.IfStatement: - IfStatementSyntax ifStmt = (IfStatementSyntax)statement; - ElseClauseSyntax? elseOpt = ifStmt.Else; - return GetFirstExcludedToken(elseOpt == null ? ifStmt.Statement : elseOpt.Statement); + return GetFirstExcludedIfStatementToken((IfStatementSyntax)statement); case SyntaxKind.LabeledStatement: return GetFirstExcludedToken(((LabeledStatementSyntax)statement).Statement); case SyntaxKind.LockStatement: @@ -452,6 +450,26 @@ internal static SyntaxToken GetFirstExcludedToken(StatementSyntax statement) } } + private static SyntaxToken GetFirstExcludedIfStatementToken(IfStatementSyntax ifStmt) + { + while (true) + { + ElseClauseSyntax? elseOpt = ifStmt.Else; + if (elseOpt is null) + { + return GetFirstExcludedToken(ifStmt.Statement); + } + if (elseOpt.Statement is IfStatementSyntax nestedIf) + { + ifStmt = nestedIf; + } + else + { + return GetFirstExcludedToken(elseOpt.Statement); + } + } + } + internal static bool IsInAnonymousFunctionOrQuery(int position, SyntaxNode lambdaExpressionOrQueryNode) { Debug.Assert(lambdaExpressionOrQueryNode.IsAnonymousFunction() || lambdaExpressionOrQueryNode.IsQuery()); diff --git a/src/Compilers/CSharp/Test/Emit/Emit/DynamicAnalysis/DynamicInstrumentationTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/DynamicAnalysis/DynamicInstrumentationTests.cs index 026652a9acfc7..dc9347a1974da 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/DynamicAnalysis/DynamicInstrumentationTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/DynamicAnalysis/DynamicInstrumentationTests.cs @@ -3095,6 +3095,78 @@ static void Test() " + checker.ExpectedOutput; var verifier = CompileAndVerify(source, expectedOutput, options: TestOptions.ReleaseExe); + verifier.VerifyIL("Class1.Method1", +@" +{ + // Code size 138 (0x8a) + .maxstack 5 + .locals init (bool[] V_0) + IL_0000: ldsfld ""bool[][] .PayloadRoot0"" + IL_0005: ldtoken ""void Class1.Method1(int)"" + IL_000a: ldelem.ref + IL_000b: stloc.0 + IL_000c: ldloc.0 + IL_000d: brtrue.s IL_0034 + IL_000f: ldsfld ""System.Guid .MVID"" + IL_0014: ldtoken ""void Class1.Method1(int)"" + IL_0019: ldtoken Source Document 0 + IL_001e: ldsfld ""bool[][] .PayloadRoot0"" + IL_0023: ldtoken ""void Class1.Method1(int)"" + IL_0028: ldelema ""bool[]"" + IL_002d: ldc.i4.7 + IL_002e: call ""bool[] Microsoft.CodeAnalysis.Runtime.Instrumentation.CreatePayload(System.Guid, int, int, ref bool[], int)"" + IL_0033: stloc.0 + IL_0034: ldloc.0 + IL_0035: ldc.i4.0 + IL_0036: ldc.i4.1 + IL_0037: stelem.i1 + IL_0038: ldloc.0 + IL_0039: ldc.i4.1 + IL_003a: ldc.i4.1 + IL_003b: stelem.i1 + IL_003c: ldstr ""Method1: x = {0}"" + IL_0041: ldarg.1 + IL_0042: box ""int"" + IL_0047: call ""string string.Format(string, object)"" + IL_004c: call ""void System.Console.WriteLine(string)"" + IL_0051: ldloc.0 + IL_0052: ldc.i4.6 + IL_0053: ldc.i4.1 + IL_0054: stelem.i1 + IL_0055: ldarg.1 + IL_0056: ldc.i4.0 + IL_0057: ble.s IL_0073 + IL_0059: ldloc.0 + IL_005a: ldc.i4.2 + IL_005b: ldc.i4.1 + IL_005c: stelem.i1 + IL_005d: ldstr ""Method1: x > 0"" + IL_0062: call ""void System.Console.WriteLine(string)"" + IL_0067: ldloc.0 + IL_0068: ldc.i4.3 + IL_0069: ldc.i4.1 + IL_006a: stelem.i1 + IL_006b: ldarg.0 + IL_006c: ldc.i4.0 + IL_006d: call ""void Class1.Method1(int)"" + IL_0072: ret + IL_0073: ldloc.0 + IL_0074: ldc.i4.5 + IL_0075: ldc.i4.1 + IL_0076: stelem.i1 + IL_0077: ldarg.1 + IL_0078: ldc.i4.0 + IL_0079: bge.s IL_0089 + IL_007b: ldloc.0 + IL_007c: ldc.i4.4 + IL_007d: ldc.i4.1 + IL_007e: stelem.i1 + IL_007f: ldstr ""Method1: x < 0"" + IL_0084: call ""void System.Console.WriteLine(string)"" + IL_0089: ret +} +"); + checker.CompleteCheck(verifier.Compilation, source); verifier.VerifyDiagnostics(); diff --git a/src/Compilers/CSharp/Test/EndToEnd/EndToEndTests.cs b/src/Compilers/CSharp/Test/EndToEnd/EndToEndTests.cs index 9fc40f7fc40b3..c46bfa8f3b768 100644 --- a/src/Compilers/CSharp/Test/EndToEnd/EndToEndTests.cs +++ b/src/Compilers/CSharp/Test/EndToEnd/EndToEndTests.cs @@ -411,9 +411,9 @@ public void NestedIfStatements() int nestingLevel = (IntPtr.Size, ExecutionConditionUtil.Configuration) switch { (4, ExecutionConfiguration.Debug) => 310, - (4, ExecutionConfiguration.Release) => 1650, + (4, ExecutionConfiguration.Release) => 1419, (8, ExecutionConfiguration.Debug) => 200, - (8, ExecutionConfiguration.Release) => 780, + (8, ExecutionConfiguration.Release) => 474, _ => throw new Exception($"Unexpected configuration {IntPtr.Size * 8}-bit {ExecutionConditionUtil.Configuration}") }; @@ -450,6 +450,95 @@ static void Main() } } + [WorkItem("https://github.com/dotnet/roslyn/issues/72393")] + [ConditionalTheory(typeof(NoIOperationValidation))] + [InlineData(2)] +#if DEBUG + [InlineData(2000)] +#else + [InlineData(5000)] +#endif + public void NestedIfElse(int n) + { + var builder = new System.Text.StringBuilder(); + builder.AppendLine(""" + #nullable enable + class Program + { + static void F(int i) + { + if (i == 0) { } + """); + for (int i = 0; i < n; i++) + { + builder.AppendLine($$""" + else if (i == {{i}}) { } + """); + } + builder.AppendLine(""" + } + } + """); + + var source = builder.ToString(); + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics(); + + var tree = comp.SyntaxTrees.Single(); + var model = comp.GetSemanticModel(tree); + var node = tree.GetRoot().DescendantNodes().OfType().Single(); + + // Avoid using ControlFlowGraphVerifier.GetControlFlowGraph() since that calls + // TestOperationVisitor.VerifySubTree() which has quadratic behavior using + // MemberSemanticModel.GetEnclosingBinderInternalWithinRoot(). + var operation = (Microsoft.CodeAnalysis.Operations.IMethodBodyOperation)model.GetOperation(node); + var graph = Microsoft.CodeAnalysis.FlowAnalysis.ControlFlowGraph.Create(operation); + + if (n == 2) + { + var symbol = model.GetDeclaredSymbol(node); + ControlFlowGraphVerifier.VerifyGraph(comp, """ + Block[B0] - Entry + Statements (0) + Next (Regular) Block[B1] + Block[B1] - Block + Predecessors: [B0] + Statements (0) + Jump if False (Regular) to Block[B2] + IBinaryOperation (BinaryOperatorKind.Equals) (OperationKind.Binary, Type: System.Boolean) (Syntax: 'i == 0') + Left: + IParameterReferenceOperation: i (OperationKind.ParameterReference, Type: System.Int32) (Syntax: 'i') + Right: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 0) (Syntax: '0') + Next (Regular) Block[B4] + Block[B2] - Block + Predecessors: [B1] + Statements (0) + Jump if False (Regular) to Block[B3] + IBinaryOperation (BinaryOperatorKind.Equals) (OperationKind.Binary, Type: System.Boolean) (Syntax: 'i == 0') + Left: + IParameterReferenceOperation: i (OperationKind.ParameterReference, Type: System.Int32) (Syntax: 'i') + Right: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 0) (Syntax: '0') + Next (Regular) Block[B4] + Block[B3] - Block + Predecessors: [B2] + Statements (0) + Jump if False (Regular) to Block[B4] + IBinaryOperation (BinaryOperatorKind.Equals) (OperationKind.Binary, Type: System.Boolean) (Syntax: 'i == 1') + Left: + IParameterReferenceOperation: i (OperationKind.ParameterReference, Type: System.Int32) (Syntax: 'i') + Right: + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') + Next (Regular) Block[B4] + Block[B4] - Exit + Predecessors: [B1] [B2] [B3*2] + Statements (0) + """, + graph, symbol); + } + } + [WorkItem(42361, "https://github.com/dotnet/roslyn/issues/42361")] [ConditionalFact(typeof(WindowsOrLinuxOnly))] public void Constraints() diff --git a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IIfStatement.cs b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IIfStatement.cs index a515cc28f9774..30dea08887385 100644 --- a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IIfStatement.cs +++ b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IIfStatement.cs @@ -3702,5 +3702,154 @@ void M(dynamic a, bool result) VerifyFlowGraphAndDiagnosticsForTest(source, expectedFlowGraph, expectedDiagnostics); } + + [CompilerTrait(CompilerFeature.IOperation, CompilerFeature.Dataflow)] + [Fact] + public void IfFlow_25() + { + string source = @" +class P +{ + void M(bool a, bool b) +/**/{ + if (a) + { + a = false; + } + else if (b) + { + b = false; + } + }/**/ +} +"; + string expectedFlowGraph = @" +Block[B0] - Entry + Statements (0) + Next (Regular) Block[B1] +Block[B1] - Block + Predecessors: [B0] + Statements (0) + Jump if False (Regular) to Block[B3] + IParameterReferenceOperation: a (OperationKind.ParameterReference, Type: System.Boolean) (Syntax: 'a') + Next (Regular) Block[B2] +Block[B2] - Block + Predecessors: [B1] + Statements (1) + IExpressionStatementOperation (OperationKind.ExpressionStatement, Type: null) (Syntax: 'a = false;') + Expression: + ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: System.Boolean) (Syntax: 'a = false') + Left: + IParameterReferenceOperation: a (OperationKind.ParameterReference, Type: System.Boolean) (Syntax: 'a') + Right: + ILiteralOperation (OperationKind.Literal, Type: System.Boolean, Constant: False) (Syntax: 'false') + Next (Regular) Block[B5] +Block[B3] - Block + Predecessors: [B1] + Statements (0) + Jump if False (Regular) to Block[B5] + IParameterReferenceOperation: b (OperationKind.ParameterReference, Type: System.Boolean) (Syntax: 'b') + Next (Regular) Block[B4] +Block[B4] - Block + Predecessors: [B3] + Statements (1) + IExpressionStatementOperation (OperationKind.ExpressionStatement, Type: null) (Syntax: 'b = false;') + Expression: + ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: System.Boolean) (Syntax: 'b = false') + Left: + IParameterReferenceOperation: b (OperationKind.ParameterReference, Type: System.Boolean) (Syntax: 'b') + Right: + ILiteralOperation (OperationKind.Literal, Type: System.Boolean, Constant: False) (Syntax: 'false') + Next (Regular) Block[B5] +Block[B5] - Exit + Predecessors: [B2] [B3] [B4] + Statements (0) +"; + var expectedDiagnostics = DiagnosticDescription.None; + + VerifyFlowGraphAndDiagnosticsForTest(source, expectedFlowGraph, expectedDiagnostics); + } + + [CompilerTrait(CompilerFeature.IOperation, CompilerFeature.Dataflow)] + [Fact] + public void IfFlow_26() + { + string source = @" +class P +{ + void M(bool a, bool b, bool c) +/**/{ + if (a) + { + a = false; + } + else if (b) + { + b = false; + } + else + { + c = false; + } + }/**/ +} +"; + string expectedFlowGraph = @" +Block[B0] - Entry + Statements (0) + Next (Regular) Block[B1] +Block[B1] - Block + Predecessors: [B0] + Statements (0) + Jump if False (Regular) to Block[B3] + IParameterReferenceOperation: a (OperationKind.ParameterReference, Type: System.Boolean) (Syntax: 'a') + Next (Regular) Block[B2] +Block[B2] - Block + Predecessors: [B1] + Statements (1) + IExpressionStatementOperation (OperationKind.ExpressionStatement, Type: null) (Syntax: 'a = false;') + Expression: + ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: System.Boolean) (Syntax: 'a = false') + Left: + IParameterReferenceOperation: a (OperationKind.ParameterReference, Type: System.Boolean) (Syntax: 'a') + Right: + ILiteralOperation (OperationKind.Literal, Type: System.Boolean, Constant: False) (Syntax: 'false') + Next (Regular) Block[B6] +Block[B3] - Block + Predecessors: [B1] + Statements (0) + Jump if False (Regular) to Block[B5] + IParameterReferenceOperation: b (OperationKind.ParameterReference, Type: System.Boolean) (Syntax: 'b') + Next (Regular) Block[B4] +Block[B4] - Block + Predecessors: [B3] + Statements (1) + IExpressionStatementOperation (OperationKind.ExpressionStatement, Type: null) (Syntax: 'b = false;') + Expression: + ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: System.Boolean) (Syntax: 'b = false') + Left: + IParameterReferenceOperation: b (OperationKind.ParameterReference, Type: System.Boolean) (Syntax: 'b') + Right: + ILiteralOperation (OperationKind.Literal, Type: System.Boolean, Constant: False) (Syntax: 'false') + Next (Regular) Block[B6] +Block[B5] - Block + Predecessors: [B3] + Statements (1) + IExpressionStatementOperation (OperationKind.ExpressionStatement, Type: null) (Syntax: 'c = false;') + Expression: + ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: System.Boolean) (Syntax: 'c = false') + Left: + IParameterReferenceOperation: c (OperationKind.ParameterReference, Type: System.Boolean) (Syntax: 'c') + Right: + ILiteralOperation (OperationKind.Literal, Type: System.Boolean, Constant: False) (Syntax: 'false') + Next (Regular) Block[B6] +Block[B6] - Exit + Predecessors: [B2] [B4] [B5] + Statements (0) +"; + var expectedDiagnostics = DiagnosticDescription.None; + + VerifyFlowGraphAndDiagnosticsForTest(source, expectedFlowGraph, expectedDiagnostics); + } } } diff --git a/src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs b/src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs index e688c62a319ae..4a2c0db260781 100644 --- a/src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs +++ b/src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs @@ -1536,52 +1536,58 @@ private void VisitMethodBodies(IBlockOperation? blockBody, IBlockOperation? expr { if (operation == _currentStatement) { - if (operation.WhenFalse == null) - { - // if (condition) - // consequence; - // - // becomes - // - // GotoIfFalse condition afterif; - // consequence; - // afterif: + // if (condition) + // consequence; + // + // becomes + // + // GotoIfFalse condition afterif; + // consequence; + // afterif: - BasicBlockBuilder? afterIf = null; - VisitConditionalBranch(operation.Condition, ref afterIf, jumpIfTrue: false); - VisitStatement(operation.WhenTrue); - AppendNewBlock(afterIf); - } - else - { - // if (condition) - // consequence; - // else - // alternative - // - // becomes - // - // GotoIfFalse condition alt; - // consequence - // goto afterif; - // alt: - // alternative; - // afterif: + // if (condition) + // consequence; + // else + // alternative + // + // becomes + // + // GotoIfFalse condition alt; + // consequence + // goto afterif; + // alt: + // alternative; + // afterif: + var afterIf = new BasicBlockBuilder(BasicBlockKind.Block); + + while (true) + { BasicBlockBuilder? whenFalse = null; VisitConditionalBranch(operation.Condition, ref whenFalse, jumpIfTrue: false); - + Debug.Assert(whenFalse is { }); VisitStatement(operation.WhenTrue); - - var afterIf = new BasicBlockBuilder(BasicBlockKind.Block); UnconditionalBranch(afterIf); AppendNewBlock(whenFalse); - VisitStatement(operation.WhenFalse); - AppendNewBlock(afterIf); + if (operation.WhenFalse is IConditionalOperation nested) + { + operation = nested; + } + else + { + break; + } } + if (operation.WhenFalse is not null) + { + VisitStatement(operation.WhenFalse); + } + + AppendNewBlock(afterIf); + return null; } else diff --git a/src/Compilers/Core/Portable/Operations/OperationMapBuilder.cs b/src/Compilers/Core/Portable/Operations/OperationMapBuilder.cs index 34abe6d500ed4..48be8601cee34 100644 --- a/src/Compilers/Core/Portable/Operations/OperationMapBuilder.cs +++ b/src/Compilers/Core/Portable/Operations/OperationMapBuilder.cs @@ -53,6 +53,27 @@ private sealed class Walker : OperationWalker return null; } + public override object? VisitConditional(IConditionalOperation operation, Dictionary argument) + { + while (true) + { + RecordOperation(operation, argument); + Visit(operation.Condition, argument); + Visit(operation.WhenTrue, argument); + if (operation.WhenFalse is IConditionalOperation nested) + { + operation = nested; + } + else + { + Visit(operation.WhenFalse, argument); + break; + } + } + + return null; + } + internal override object? VisitNoneOperation(IOperation operation, Dictionary argument) { // OperationWalker skips these nodes by default, to avoid having public consumers deal with NoneOperation.