Fix EnC debug information emitted for patterns

Update calculation of syntax offset to account for a new case when a node (a switch expression) that is associated with a variable, closure or lambda may share start offset with other node of the same kind (`expr switch { … } switch { … }`). Use the offset of the `switch` keyword instead of the starting offset of the expression to disambiguate.

Assign ordinals to variables synthesized for storing pattern values across cases. This is required to support complex patterns since we can no longer rely on the type of these variables to be distinct. This will require follow up in the IDE to disallow updating/adding/reordering the case clauses of switch expression which there an active statement is present within the switch statement. If the cases are unmodified the compiler guarantees that the order in which the synthesized variables are generated remains the same, so we can map the variables using their ordinal.

Mark all variables synthesized during lowering of switch expression as short-lived. Their lifespan is limited to the switch expression, which does not include a sequence point.

Disallow editing methods that contain switch expression. This is necessary until bugs dotnet#37232, dotnet#37237 are fixed.

src/Compilers/CSharp/Portable/CodeGen/CodeGenerator.cs

@@ -157,7 +157,7 @@ private LocalDefinition LazyReturnTemp
                     var bodySyntax = _methodBodySyntaxOpt;
                     if (_ilEmitStyle == ILEmitStyle.Debug && bodySyntax != null)
                     {
-                        int syntaxOffset = _method.CalculateLocalSyntaxOffset(bodySyntax.SpanStart, bodySyntax.SyntaxTree);
+                        int syntaxOffset = _method.CalculateLocalSyntaxOffset(LambdaUtilities.GetDeclaratorPosition(bodySyntax), bodySyntax.SyntaxTree);
                         var localSymbol = new SynthesizedLocal(_method, _method.ReturnTypeWithAnnotations, SynthesizedLocalKind.FunctionReturnValue, bodySyntax);
 
                         result = _builder.LocalSlotManager.DeclareLocal(

src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs

@@ -1435,13 +1435,8 @@ private string GetLocalDebugName(ILocalSymbolInternal local, out LocalDebugId lo
             if (_ilEmitStyle == ILEmitStyle.Debug)
             {
                 var syntax = local.GetDeclaratorSyntax();
-                int syntaxOffset = _method.CalculateLocalSyntaxOffset(syntax.SpanStart, syntax.SyntaxTree);
-
-                // Synthesized locals emitted for switch case patterns are all associated with the switch statement 
-                // and have distinct types. We use their types to match them, not the ordinal as the ordinal might
-                // change if switch cases are reordered.
-                int ordinal = (localKind != SynthesizedLocalKind.SwitchCasePatternMatching) ?
-                    _synthesizedLocalOrdinals.AssignLocalOrdinal(localKind, syntaxOffset) : 0;
+                int syntaxOffset = _method.CalculateLocalSyntaxOffset(LambdaUtilities.GetDeclaratorPosition(syntax), syntax.SyntaxTree);
+                int ordinal = _synthesizedLocalOrdinals.AssignLocalOrdinal(localKind, syntaxOffset);
 
                 // user-defined locals should have 0 ordinal:
                 Debug.Assert(ordinal == 0 || localKind != SynthesizedLocalKind.UserDefined);

src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpLambdaSyntaxFacts.cs

@@ -13,16 +13,12 @@ private CSharpLambdaSyntaxFacts()
         }
 
         public override SyntaxNode GetLambda(SyntaxNode lambdaOrLambdaBodySyntax)
-        {
-            return LambdaUtilities.GetLambda(lambdaOrLambdaBodySyntax);
-        }
+            => LambdaUtilities.GetLambda(lambdaOrLambdaBodySyntax);
 
-        public override SyntaxNode TryGetCorrespondingLambdaBody(
-            SyntaxNode previousLambdaSyntax,
-            SyntaxNode lambdaOrLambdaBodySyntax)
-        {
-            return LambdaUtilities.TryGetCorrespondingLambdaBody(
-                lambdaOrLambdaBodySyntax, previousLambdaSyntax);
-        }
+        public override SyntaxNode TryGetCorrespondingLambdaBody(SyntaxNode previousLambdaSyntax, SyntaxNode lambdaOrLambdaBodySyntax)
+            => LambdaUtilities.TryGetCorrespondingLambdaBody(lambdaOrLambdaBodySyntax, previousLambdaSyntax);
+
+        public override int GetDeclaratorPosition(SyntaxNode node)
+            => LambdaUtilities.GetDeclaratorPosition(node);
     }
 }

src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs

@@ -535,7 +535,7 @@ internal DebugId GetClosureId(SyntaxNode syntax, ArrayBuilder<ClosureDebugInfo>
                     closureId = new DebugId(closureDebugInfo.Count, _compilationState.ModuleBuilderOpt.CurrentGenerationOrdinal);
                 }
 
-                int syntaxOffset = _topLevelMethod.CalculateLocalSyntaxOffset(syntax.SpanStart, syntax.SyntaxTree);
+                int syntaxOffset = _topLevelMethod.CalculateLocalSyntaxOffset(LambdaUtilities.GetDeclaratorPosition(syntax), syntax.SyntaxTree);
                 closureDebugInfo.Add(new ClosureDebugInfo(syntaxOffset, closureId));
 
                 return closureId;

src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs

@@ -1388,7 +1388,7 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos
                 lambdaId = new DebugId(_lambdaDebugInfoBuilder.Count, CompilationState.ModuleBuilderOpt.CurrentGenerationOrdinal);
             }
 
-            int syntaxOffset = _topLevelMethod.CalculateLocalSyntaxOffset(lambdaOrLambdaBodySyntax.SpanStart, lambdaOrLambdaBodySyntax.SyntaxTree);
+            int syntaxOffset = _topLevelMethod.CalculateLocalSyntaxOffset(LambdaUtilities.GetDeclaratorPosition(lambdaOrLambdaBodySyntax), lambdaOrLambdaBodySyntax.SyntaxTree);
             _lambdaDebugInfoBuilder.Add(new LambdaDebugInfo(syntaxOffset, lambdaId, closureOrdinal));
             return lambdaId;
         }

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BasePatternSwitchLocalRewriter.cs

@@ -17,7 +17,7 @@ internal partial class LocalRewriter
         /// <summary>
         /// A common base class for lowering the pattern switch statement and the pattern switch expression.
         /// </summary>
-        private class BaseSwitchLocalRewriter : PatternLocalRewriter
+        private abstract class BaseSwitchLocalRewriter : PatternLocalRewriter
         {
             /// <summary>
             /// Map from switch section's syntax to the lowered code for the section. The code for a section
@@ -37,27 +37,19 @@ private class BaseSwitchLocalRewriter : PatternLocalRewriter
             /// </summary>
             private readonly PooledDictionary<BoundDecisionDagNode, LabelSymbol> _dagNodeLabels = PooledDictionary<BoundDecisionDagNode, LabelSymbol>.GetInstance();
 
-            /// <summary>
-            /// True if we are translating a switch statement. This affects sequence points (a when clause gets
-            /// a sequence point in a switch statement, but not in a switch expression).
-            /// </summary>
-            private readonly bool _isSwitchStatement;
-
             protected BaseSwitchLocalRewriter(
                 SyntaxNode node,
                 LocalRewriter localRewriter,
-                ImmutableArray<SyntaxNode> arms,
-                bool isSwitchStatement)
+                ImmutableArray<SyntaxNode> arms)
                 : base(node, localRewriter)
             {
-                this._isSwitchStatement = isSwitchStatement;
                 foreach (var arm in arms)
                 {
                     var armBuilder = ArrayBuilder<BoundStatement>.GetInstance();
 
                     // We start each switch block of a switch statement with a hidden sequence point so that
                     // we do not appear to be in the previous switch block when we begin.
-                    if (isSwitchStatement)
+                    if (IsSwitchStatement)
                         armBuilder.Add(_factory.HiddenSequencePoint());
 
                     _switchArms.Add(arm, armBuilder);
@@ -289,7 +281,7 @@ protected BoundDecisionDag ShareTempsIfPossibleAndEvaluateInput(
                 // In a switch statement, there is a hidden sequence point after evaluating the input at the start of
                 // the code to handle the decision dag. This is necessary so that jumps back from a `when` clause into
                 // the decision dag do not appear to jump back up to the enclosing construct.
-                if (_isSwitchStatement)
+                if (IsSwitchStatement)
                     result.Add(_factory.HiddenSequencePoint());
 
                 return decisionDag;
@@ -303,9 +295,9 @@ protected BoundDecisionDag ShareTempsIfPossibleAndEvaluateInput(
                 Debug.Assert(this._loweredDecisionDag.IsEmpty());
                 ComputeLabelSet(decisionDag);
                 LowerDecisionDagCore(decisionDag);
-                ImmutableArray<BoundStatement> loweredDag = this._loweredDecisionDag.ToImmutableAndFree();
-                ImmutableDictionary<SyntaxNode, ImmutableArray<BoundStatement>> switchSections = this._switchArms.ToImmutableDictionary(kv => kv.Key, kv => kv.Value.ToImmutableAndFree());
-                this._switchArms.Clear();
+                ImmutableArray<BoundStatement> loweredDag = _loweredDecisionDag.ToImmutableAndFree();
+                var switchSections = _switchArms.ToImmutableDictionary(kv => kv.Key, kv => kv.Value.ToImmutableAndFree());
+                _switchArms.Clear();
                 return (loweredDag, switchSections);
             }
 
@@ -343,7 +335,7 @@ private void LowerDecisionDagCore(BoundDecisionDag decisionDag)
                         continue;
                     }
 
-                    if (this._dagNodeLabels.TryGetValue(node, out LabelSymbol label))
+                    if (_dagNodeLabels.TryGetValue(node, out LabelSymbol label))
                     {
                         _loweredDecisionDag.Add(_factory.Label(label));
                     }
@@ -581,7 +573,7 @@ private void LowerWhenClause(BoundWhenDecisionDagNode whenClause)
                     BoundStatement conditionalGoto = _factory.ConditionalGoto(_localRewriter.VisitExpression(whenClause.WhenExpression), trueLabel, jumpIfTrue: true);
 
                     // Only add instrumentation (such as a sequence point) if the node is not compiler-generated.
-                    if (_isSwitchStatement && !whenClause.WhenExpression.WasCompilerGenerated && _localRewriter.Instrument)
+                    if (IsSwitchStatement && !whenClause.WhenExpression.WasCompilerGenerated && _localRewriter.Instrument)
                     {
                         conditionalGoto = _localRewriter._instrumenter.InstrumentSwitchWhenClauseConditionalGotoBody(whenClause.WhenExpression, conditionalGoto);
                     }
@@ -592,7 +584,7 @@ private void LowerWhenClause(BoundWhenDecisionDagNode whenClause)
 
                     // We hide the jump back into the decision dag, as it is not logically part of the when clause
                     BoundStatement jump = _factory.Goto(GetDagNodeLabel(whenFalse));
-                    sectionBuilder.Add(_isSwitchStatement ? _factory.HiddenSequencePoint(jump) : jump);
+                    sectionBuilder.Add(IsSwitchStatement ? _factory.HiddenSequencePoint(jump) : jump);
                 }
                 else
                 {
@@ -618,7 +610,7 @@ private void LowerDecisionDagNode(BoundDecisionDagNode node, BoundDecisionDagNod
                             // We add a hidden sequence point after the evaluation's side-effect, which may be a call out
                             // to user code such as `Deconstruct` or a property get, to permit edit-and-continue to
                             // synchronize on changes.
-                            if (_isSwitchStatement)
+                            if (IsSwitchStatement)
                                 _loweredDecisionDag.Add(_factory.HiddenSequencePoint());
 
                             if (nextNode != evaluationNode.Next)

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_IsPatternOperator.cs

@@ -18,7 +18,7 @@ public override BoundNode VisitIsPatternExpression(BoundIsPatternExpression node
             return result;
         }
 
-        private class IsPatternExpressionLocalRewriter : PatternLocalRewriter
+        private sealed class IsPatternExpressionLocalRewriter : PatternLocalRewriter
         {
             /// <summary>
             /// Accumulates side-effects that come before the next conjunct.
@@ -35,10 +35,12 @@ private class IsPatternExpressionLocalRewriter : PatternLocalRewriter
             public IsPatternExpressionLocalRewriter(SyntaxNode node, LocalRewriter localRewriter)
                 : base(node, localRewriter)
             {
-                this._conjunctBuilder = ArrayBuilder<BoundExpression>.GetInstance();
-                this._sideEffectBuilder = ArrayBuilder<BoundExpression>.GetInstance();
+                _conjunctBuilder = ArrayBuilder<BoundExpression>.GetInstance();
+                _sideEffectBuilder = ArrayBuilder<BoundExpression>.GetInstance();
             }
 
+            protected override bool IsSwitchStatement => false;
+
             public new void Free()
             {
                 _conjunctBuilder.Free();

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_PatternSwitchStatement.cs

@@ -16,24 +16,26 @@ internal partial class LocalRewriter
     {
         public override BoundNode VisitSwitchStatement(BoundSwitchStatement node)
         {
-            return SwitchLocalRewriter.Rewrite(this, node);
+            return SwitchStatementLocalRewriter.Rewrite(this, node);
         }
 
-        private class SwitchLocalRewriter : BaseSwitchLocalRewriter
+        private sealed class SwitchStatementLocalRewriter : BaseSwitchLocalRewriter
         {
+            /// <summary>
+            /// A map from section syntax to the first label in that section.
+            /// </summary>
+            private readonly Dictionary<SyntaxNode, LabelSymbol> _sectionLabels = PooledDictionary<SyntaxNode, LabelSymbol>.GetInstance();
+
+            protected override bool IsSwitchStatement => true;
+
             public static BoundStatement Rewrite(LocalRewriter localRewriter, BoundSwitchStatement node)
             {
-                var rewriter = new SwitchLocalRewriter(node, localRewriter);
+                var rewriter = new SwitchStatementLocalRewriter(node, localRewriter);
                 BoundStatement result = rewriter.LowerSwitchStatement(node);
                 rewriter.Free();
                 return result;
             }
 
-            /// <summary>
-            /// A map from section syntax to the first label in that section.
-            /// </summary>
-            private Dictionary<SyntaxNode, LabelSymbol> _sectionLabels = PooledDictionary<SyntaxNode, LabelSymbol>.GetInstance();
-
             /// <summary>
             /// We revise the returned label for a leaf so that all leaves in the same switch section are given the same label.
             /// This enables the switch emitter to produce better code.
@@ -62,8 +64,8 @@ protected override LabelSymbol GetDagNodeLabel(BoundDecisionDagNode dag)
                 return result;
             }
 
-            private SwitchLocalRewriter(BoundSwitchStatement node, LocalRewriter localRewriter)
-                : base(node.Syntax, localRewriter, node.SwitchSections.SelectAsArray(section => section.Syntax), isSwitchStatement: true)
+            private SwitchStatementLocalRewriter(BoundSwitchStatement node, LocalRewriter localRewriter)
+                : base(node.Syntax, localRewriter, node.SwitchSections.SelectAsArray(section => section.Syntax))
             {
             }
 

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs

@@ -16,35 +16,47 @@ internal sealed partial class LocalRewriter
         /// <summary>
         /// A common base class for lowering constructs that use pattern-matching.
         /// </summary>
-        private class PatternLocalRewriter
+        private abstract class PatternLocalRewriter
         {
             protected readonly LocalRewriter _localRewriter;
             protected readonly SyntheticBoundNodeFactory _factory;
             protected readonly DagTempAllocator _tempAllocator;
 
             public PatternLocalRewriter(SyntaxNode node, LocalRewriter localRewriter)
             {
-                this._localRewriter = localRewriter;
-                this._factory = localRewriter._factory;
-                this._tempAllocator = new DagTempAllocator(_factory, node);
+                _localRewriter = localRewriter;
+                _factory = localRewriter._factory;
+                _tempAllocator = new DagTempAllocator(_factory, node, IsSwitchStatement);
             }
 
+            /// <summary>
+            /// True if this is a rewriter for a switch statement. This affects 
+            /// - sequence points
+            ///   When clause gets a sequence point in a switch statement, but not in a switch expression.
+            /// - synthesized local variable kind
+            ///   The temp variables must be long lived in a switch statement since their lifetime spans across sequence points.
+            /// </summary>
+            protected abstract bool IsSwitchStatement { get; }
+
             public void Free()
             {
                 _tempAllocator.Free();
             }
 
-            public class DagTempAllocator
+            public sealed class DagTempAllocator
             {
                 private readonly SyntheticBoundNodeFactory _factory;
                 private readonly PooledDictionary<BoundDagTemp, BoundExpression> _map = PooledDictionary<BoundDagTemp, BoundExpression>.GetInstance();
                 private readonly ArrayBuilder<LocalSymbol> _temps = ArrayBuilder<LocalSymbol>.GetInstance();
                 private readonly SyntaxNode _node;
 
-                public DagTempAllocator(SyntheticBoundNodeFactory factory, SyntaxNode node)
+                private readonly bool _isSwitchStatement;
+
+                public DagTempAllocator(SyntheticBoundNodeFactory factory, SyntaxNode node, bool isSwitchStatement)
                 {
-                    this._factory = factory;
-                    this._node = node;
+                    _factory = factory;
+                    _node = node;
+                    _isSwitchStatement = isSwitchStatement;
                 }
 
                 public void Free()
@@ -76,7 +88,8 @@ public BoundExpression GetTemp(BoundDagTemp dagTemp)
                 {
                     if (!_map.TryGetValue(dagTemp, out BoundExpression result))
                     {
-                        LocalSymbol temp = _factory.SynthesizedLocal(dagTemp.Type, syntax: _node, kind: SynthesizedLocalKind.SwitchCasePatternMatching);
+                        var kind = _isSwitchStatement ? SynthesizedLocalKind.SwitchCasePatternMatching : SynthesizedLocalKind.LoweringTemp;
+                        LocalSymbol temp = _factory.SynthesizedLocal(dagTemp.Type, syntax: _node, kind: kind);
                         result = _factory.Local(temp);
                         _map.Add(dagTemp, result);
                         _temps.Add(temp);

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_SwitchExpression.cs

@@ -20,14 +20,16 @@ public override BoundNode VisitConvertedSwitchExpression(BoundConvertedSwitchExp
             return SwitchExpressionLocalRewriter.Rewrite(this, node);
         }
 
-        private class SwitchExpressionLocalRewriter : BaseSwitchLocalRewriter
+        private sealed class SwitchExpressionLocalRewriter : BaseSwitchLocalRewriter
         {
             private SwitchExpressionLocalRewriter(BoundConvertedSwitchExpression node, LocalRewriter localRewriter)
-                : base(node.Syntax, localRewriter, node.SwitchArms.SelectAsArray(arm => arm.Syntax), isSwitchStatement: false)
+                : base(node.Syntax, localRewriter, node.SwitchArms.SelectAsArray(arm => arm.Syntax))
             {
             }
 
-            public static BoundExpression Rewrite(LocalRewriter localRewriter, BoundConvertedSwitchExpression node)
+            protected override bool IsSwitchStatement => false;
+
+            public static BoundExpression Rewrite(LocalRewriter localRewriter, BoundSwitchExpression node)
             {
                 var rewriter = new SwitchExpressionLocalRewriter(node, localRewriter);
                 BoundExpression result = rewriter.LowerSwitchExpression(node);
@@ -55,7 +57,7 @@ private BoundExpression LowerSwitchExpression(BoundConvertedSwitchExpression nod
                 // decision tree, so the code in result is unreachable at this point.
 
                 // Lower each switch expression arm
-                LocalSymbol resultTemp = _factory.SynthesizedLocal(node.Type, node.Syntax, kind: SynthesizedLocalKind.SwitchCasePatternMatching);
+                LocalSymbol resultTemp = _factory.SynthesizedLocal(node.Type, node.Syntax, kind: SynthesizedLocalKind.LoweringTemp);
                 LabelSymbol afterSwitchExpression = _factory.GenerateLabel("afterSwitchExpression");
                 foreach (BoundSwitchExpressionArm arm in node.SwitchArms)
                 {

src/Compilers/CSharp/Portable/Lowering/SpillSequenceSpiller.cs

@@ -543,7 +543,6 @@ public override BoundNode VisitAwaitExpression(BoundAwaitExpression node)
 
         public override BoundNode VisitSpillSequence(BoundSpillSequence node)
         {
-            Debug.Assert(node.Locals.All(l => l.SynthesizedKind.IsLongLived()));
             var builder = new BoundSpillSequenceBuilder();
 
             // Ensure later errors (e.g. in async rewriting) are associated with the correct node.

src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs

@@ -471,7 +471,7 @@ private BoundExpression HoistRefInitialization(SynthesizedLocal local, BoundAssi
             if (F.Compilation.Options.OptimizationLevel == OptimizationLevel.Debug)
             {
                 awaitSyntaxOpt = (AwaitExpressionSyntax)local.GetDeclaratorSyntax();
-                syntaxOffset = this.OriginalMethod.CalculateLocalSyntaxOffset(awaitSyntaxOpt.SpanStart, awaitSyntaxOpt.SyntaxTree);
+                syntaxOffset = OriginalMethod.CalculateLocalSyntaxOffset(LambdaUtilities.GetDeclaratorPosition(awaitSyntaxOpt), awaitSyntaxOpt.SyntaxTree);
             }
             else
             {