Debug: rework for expressions stepping

docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@@ -81,6 +81,7 @@
 * Added warning FS3884 when a function or delegate value is used as an interpolated string argument. ([PR #19289](https://github.com/dotnet/fsharp/pull/19289))
 * Symbols: add ObsoleteDiagnosticInfo ([PR #19359](https://github.com/dotnet/fsharp/pull/19359))
 * Add `#version;;` directive to F# Interactive to display version and environment information. ([Issue #13307](https://github.com/dotnet/fsharp/issues/13307), [PR #19332](https://github.com/dotnet/fsharp/pull/19332))
+* Debug: rework for expressions stepping ([PR #19894](https://github.com/dotnet/fsharp/pull/19894))
 
 ### Changed
 

src/Compiler/Checking/Expressions/CheckExpressions.fs

@@ -8270,8 +8270,8 @@ and TcForEachExpr cenv overallTy env tpenv (seqExprOnly, isFromSource, synPat, s
     let mEnumExpr = synEnumExpr.Range
     let mFor = match spFor with DebugPointAtFor.Yes mStart -> mStart | DebugPointAtFor.No -> mEnumExpr
     let mIn = match spIn with DebugPointAtInOrTo.Yes mStart -> mStart | DebugPointAtInOrTo.No -> mBodyExpr
-    let spEnumExpr = DebugPointAtBinding.Yes mEnumExpr
-    let spForBind = match spFor with DebugPointAtFor.Yes m -> DebugPointAtBinding.Yes m | DebugPointAtFor.No -> DebugPointAtBinding.NoneAtSticky
+    let spEnumExpr = match spFor with DebugPointAtFor.Yes _ -> DebugPointAtBinding.Yes mEnumExpr | DebugPointAtFor.No -> DebugPointAtBinding.NoneAtSticky
+    let spForBind = match spFor with DebugPointAtFor.Yes _ -> DebugPointAtBinding.Yes(unionRanges mFor mPat) | DebugPointAtFor.No -> DebugPointAtBinding.NoneAtInvisible
     let spInAsWhile = match spIn with DebugPointAtInOrTo.Yes m -> DebugPointAtWhile.Yes m | DebugPointAtInOrTo.No -> DebugPointAtWhile.No
 
     // Check the expression being enumerated
@@ -8295,10 +8295,10 @@ and TcForEachExpr cenv overallTy env tpenv (seqExprOnly, isFromSource, synPat, s
             let elemTy = destArrayTy g enumExprTy
 
             // Evaluate the array index lookup
-            let bodyExprFixup elemVar bodyExpr = mkInvisibleLet mIn elemVar (mkLdelem g mIn elemTy arrExpr idxExpr) bodyExpr
+            let bodyExprFixup elemVar bodyExpr = mkLet spForBind mFor elemVar (mkLdelem g mIn elemTy arrExpr idxExpr) bodyExpr
 
             // Evaluate the array expression once and put it in arrVar
-            let overallExprFixup overallExpr = mkLet spForBind mFor arrVar enumExpr overallExpr
+            let overallExprFixup overallExpr = mkLet spEnumExpr mEnumExpr arrVar enumExpr overallExpr
 
             // Ask for a loop over integers for the given range
             (elemTy, bodyExprFixup, overallExprFixup, Choice2Of3 (idxVar, mkZero g mFor, mkDecr g mFor (mkLdlen g mFor arrExpr)))
@@ -8316,12 +8316,12 @@ and TcForEachExpr cenv overallTy env tpenv (seqExprOnly, isFromSource, synPat, s
                 // Evaluate the span index lookup
                 let bodyExprFixup elemVar bodyExpr =
                     let elemAddrVar, _ = mkCompGenLocal mIn "addr" elemAddrTy
-                    let e = mkInvisibleLet mIn elemVar (mkAddrGet mIn (mkLocalValRef elemAddrVar)) bodyExpr
+                    let e = mkLet spForBind mFor elemVar (mkAddrGet mIn (mkLocalValRef elemAddrVar)) bodyExpr
                     let getItemCallExpr, _ = BuildMethodCall tcVal g cenv.amap PossiblyMutates mWholeExpr true getItemMethInfo ValUseFlag.NormalValUse [] [ spanExpr ] [ idxExpr ] None
                     mkInvisibleLet mIn elemAddrVar getItemCallExpr e
 
                 // Evaluate the span expression once and put it in spanVar
-                let overallExprFixup overallExpr = mkLet spForBind mFor spanVar enumExpr overallExpr
+                let overallExprFixup overallExpr = mkLet spEnumExpr mEnumExpr spanVar enumExpr overallExpr
 
                 let getLengthCallExpr, _ = BuildMethodCall tcVal g cenv.amap PossiblyMutates mWholeExpr true getLengthMethInfo ValUseFlag.NormalValUse [] [ spanExpr ] [] None
 
@@ -8410,13 +8410,13 @@ and TcForEachExpr cenv overallTy env tpenv (seqExprOnly, isFromSource, synPat, s
         | Choice3Of3(enumerableVar, enumeratorVar, _, getEnumExpr, _, guardExpr, currentExpr) ->
 
             // This compiled for must be matched EXACTLY by CompiledForEachExpr
-            mkLet spForBind mFor enumerableVar enumExpr
-              (mkLet spEnumExpr mFor enumeratorVar getEnumExpr
+            mkLet spEnumExpr mEnumExpr enumerableVar enumExpr
+              (mkInvisibleLet mFor enumeratorVar getEnumExpr
                    (mkTryFinally g
                        (mkWhile g
                            (spInAsWhile,
                             WhileLoopForCompiledForEachExprMarker, guardExpr,
-                            mkInvisibleLet mIn elemVar currentExpr bodyExpr,
+                            mkLet spForBind mIn elemVar currentExpr bodyExpr,
                             mFor),
                         BuildDisposableCleanup cenv env mWholeExpr enumeratorVar,
                         mFor, g.unit_ty, DebugPointAtTry.No, DebugPointAtFinally.No)))

src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs

@@ -387,7 +387,7 @@ let TcSequenceExpression (cenv: TcFileState) env tpenv comp (overallTy: OverallT
 
             Some(resultExpr, tpenv)
 
-        | SynExpr.YieldOrReturn(flags = (isYield, _); expr = synYieldExpr; trivia = { YieldOrReturnKeyword = m }) ->
+        | SynExpr.YieldOrReturn(flags = (isYield, _); expr = synYieldExpr; range = m) ->
             let env = { env with eIsControlFlow = false }
             let genResultTy = NewInferenceType g
 
@@ -404,7 +404,7 @@ let TcSequenceExpression (cenv: TcFileState) env tpenv comp (overallTy: OverallT
                 if IsControlFlowExpression synYieldExpr then
                     resultExpr
                 else
-                    mkDebugPoint synYieldExpr.Range resultExpr
+                    mkDebugPoint m resultExpr
 
             Some(resultExpr, tpenv)
 

src/Compiler/CodeGen/IlxGen.fs

@@ -3172,7 +3172,11 @@ and GenExprAux (cenv: cenv) (cgbuf: CodeGenBuffer) eenv expr (sequel: sequel) =
         | Expr.Match _ -> GenLinearExpr cenv cgbuf eenv expr sequel false id |> ignore<FakeUnit>
 
         | Expr.DebugPoint(DebugPointAtLeafExpr.Yes m, innerExpr) ->
-            CG.EmitDebugPoint cgbuf m
+            if equals m range0 then
+                cgbuf.EmitStartOfHiddenCode()
+            else
+                CG.EmitDebugPoint cgbuf m
+
             GenExpr cenv cgbuf eenv innerExpr sequel
 
         | Expr.Const(c, m, ty) -> GenConstant cenv cgbuf eenv (c, m, ty) sequel
@@ -3733,7 +3737,11 @@ and GenLinearExpr cenv cgbuf eenv expr sequel preSteps (contf: FakeUnit -> FakeU
                          Fake))
 
     | Expr.DebugPoint(DebugPointAtLeafExpr.Yes m, innerExpr) ->
-        CG.EmitDebugPoint cgbuf m
+        if equals m range0 then
+            cgbuf.EmitStartOfHiddenCode()
+        else
+            CG.EmitDebugPoint cgbuf m
+
         GenLinearExpr cenv cgbuf eenv innerExpr sequel true contf
 
     | LinearOpExpr(TOp.UnionCase c, tyargs, argsFront, argLast, m) ->
@@ -5280,6 +5288,7 @@ and GenIntegerForLoop cenv cgbuf eenv (spFor, spTo, v, e1, dir, e2, loopBody, m)
     GenExpr cenv cgbuf eenvinner loopBody discard
 
     //    v++ or v--
+    cgbuf.EmitStartOfHiddenCode()
     GenGetLocalVal cenv cgbuf eenvinner e2.Range v None
 
     CG.EmitInstr cgbuf (pop 0) (Push [ g.ilg.typ_Int32 ]) (mkLdcInt32 1)

src/Compiler/Optimize/LowerComputedCollections.fs

@@ -339,12 +339,12 @@ module List =
 
                 let loop =
                     mkLoop (fun _idxVar loopVar ->
-                        let body =
-                            body
-                            |> Option.map (fun (loopVal, body) -> mkInvisibleLet m loopVal loopVar body)
-                            |> Option.defaultValue loopVar
-
-                        mkCallCollectorAdd tcVal g reader mBody collector body)
+                        match body with
+                        | Some (loopVal, body) ->
+                            mkInvisibleLet m loopVal loopVar
+                                (Expr.DebugPoint (DebugPointAtLeafExpr.Yes mFor, mkCallCollectorAdd tcVal g reader mBody collector body))
+                        | None ->
+                            mkCallCollectorAdd tcVal g reader mBody collector loopVar)
 
                 let close = mkCallCollectorClose tcVal g reader mBody collector
                 mkSequential m loop close
@@ -504,12 +504,13 @@ module Array =
             mkCompGenLetIn mFor "array" arrayTy (mkNewArray count) (fun (_, array) ->
                 let loop =
                     mkLoop (fun idxVar loopVar ->
-                        let body =
-                            body
-                            |> Option.map (fun (loopVal, body) -> mkInvisibleLet mBody loopVal loopVar body)
-                            |> Option.defaultValue loopVar
+                        let mkStore elem = mkAsmExpr ([stelem], [], [array; convToNativeInt NoCheckOvf idxVar; elem], [], mBody)
 
-                        mkAsmExpr ([stelem], [], [array; convToNativeInt NoCheckOvf idxVar; body], [], mBody))
+                        match body with
+                        | Some (loopVal, body) ->
+                            mkInvisibleLet mBody loopVal loopVar (Expr.DebugPoint (DebugPointAtLeafExpr.Yes mFor, mkStore body))
+                        | None ->
+                            mkStore loopVar)
 
                 mkSequential m loop array)
 

src/Compiler/Optimize/Optimizer.fs

@@ -2873,11 +2873,14 @@ and OptimizeLinearExpr cenv env expr contf =
       let e1R, e1info = OptimizeExpr cenv env e1 
 
       OptimizeLinearExpr cenv env e2 (contf << (fun (e2R, e2info) -> 
-        if (flag = NormalSeq) && 
-           // Always eliminate '(); expr' sequences, even in debug code, to ensure that 
-           // conditional method calls don't leave a dangling breakpoint (see FSharp 1.0 bug 6034)
-           (cenv.settings.EliminateSequential || (match stripDebugPoints e1R with Expr.Const (Const.Unit, _, _) -> true | _ -> false)) && 
-           not e1info.HasEffect then 
+        if (flag = NormalSeq) &&
+           // Drop bare (compiler-generated) units always; keep a debug-pointed unit in debug code so
+           // it stays steppable (a unit without one must go, else a dangling breakpoint - FSharp 1.0 bug 6034).
+           (cenv.settings.EliminateSequential ||
+            (match e1R with
+             | Expr.DebugPoint(DebugPointAtLeafExpr.Yes _, _) -> false
+             | _ -> match stripDebugPoints e1R with Expr.Const (Const.Unit, _, _) -> true | _ -> false)) &&
+           not e1info.HasEffect then
             e2R, e2info
         else 
             Expr.Sequential (e1R, e2R, flag, m), 

src/Compiler/TypedTree/TypedTreeOps.Transforms.fs

@@ -1254,7 +1254,7 @@ module internal TupleCompilation =
               Let(enumeratorVar,
                   GetEnumeratorCall enumerableVar2,
                   _enumeratorBind,
-                  TryFinally(WhileLoopForCompiledForEachExpr(spInWhile, _, (Let(elemVar, _, _, bodyExpr) as elemLet), _), _))) when
+                  TryFinally(WhileLoopForCompiledForEachExpr(spInWhile, _, (Let(elemVar, _, spElem, bodyExpr) as elemLet), _), _))) when
             // Apply correctness conditions to ensure this really is a compiled for-each expression.
             valRefEq g (mkLocalValRef enumerableVar) enumerableVar2
             && enumerableVar.IsCompilerGenerated
@@ -1271,7 +1271,7 @@ module internal TupleCompilation =
             let mIn = elemLet.Range
 
             let mFor =
-                match spFor with
+                match spElem with
                 | DebugPointAtBinding.Yes mFor -> mFor
                 | _ -> enumerableExpr.Range
 
@@ -1287,7 +1287,7 @@ module internal TupleCompilation =
 
             let enumerableTy = tyOfExpr g enumerableExpr
 
-            ValueSome(enumerableTy, enumerableExpr, elemVar, bodyExpr, (mBody, spFor, spIn, mFor, mIn, spInWhile, mWholeExpr))
+            ValueSome(enumerableTy, enumerableExpr, elemVar, bodyExpr, (mBody, spFor, spElem, spIn, mFor, mIn, spInWhile, mWholeExpr))
         | _ -> ValueNone
 
     [<return: Struct>]
@@ -2147,7 +2147,7 @@ module internal TupleCompilation =
         match option, expr with
         | _, CompiledInt32RangeForEachExpr g (startExpr, (1 | -1 as step), finishExpr, elemVar, bodyExpr, ranges) ->
 
-            let _mBody, spFor, spIn, _mFor, _mIn, _spInWhile, mWholeExpr = ranges
+            let _mBody, spFor, _spElem, spIn, _mFor, _mIn, _spInWhile, mWholeExpr = ranges
 
             let spFor =
                 match spFor with
@@ -2164,13 +2164,13 @@ module internal TupleCompilation =
                      ValueNone)
             with
             | ValueSome(rangeTy, (start, step, finish)) ->
-                let mBody, _spFor, _spIn, mFor, mIn, spInWhile, _mWhole = ranges
+                let mBody, _spFor, _spElem, _spIn, mFor, mIn, spInWhile, _mWhole = ranges
 
                 mkOptimizedRangeLoop g (mBody, mFor, mIn, spInWhile) (rangeTy, enumerableExpr) (start, step, finish) (fun _count mkLoop ->
                     mkLoop (fun _idxVar loopVar -> mkInvisibleLet elemVar.Range elemVar loopVar bodyExpr))
             | ValueNone ->
 
-                let mBody, spFor, spIn, mFor, mIn, spInWhile, mWholeExpr = ranges
+                let mBody, spFor, spElem, spIn, mFor, mIn, spInWhile, mWholeExpr = ranges
 
                 if isStringTy g enumerableTy then
                     // type is string, optimize for expression as:
@@ -2189,7 +2189,7 @@ module internal TupleCompilation =
                     let finishExpr = mkDecr g mFor lengthExpr
                     // for compat reasons, loop item over string is sometimes object, not char
                     let loopItemExpr = mkCoerceIfNeeded g elemVar.Type g.char_ty charExpr
-                    let bodyExpr = mkInvisibleLet mIn elemVar loopItemExpr bodyExpr
+                    let bodyExpr = mkLet spElem mFor elemVar loopItemExpr bodyExpr
 
                     let forExpr =
                         mkFastForLoop g (DebugPointAtFor.No, spIn, mWholeExpr, idxVar, startExpr, true, finishExpr, bodyExpr)
@@ -2223,18 +2223,16 @@ module internal TupleCompilation =
                     let tailOrNullExpr =
                         mkUnionCaseFieldGetUnprovenViaExprAddr (currentExpr, g.cons_ucref, [ elemTy ], IndexTail, mIn)
 
-                    let bodyExpr =
-                        mkInvisibleLet
+                    let loopStep =
+                        mkSequential
                             mIn
-                            elemVar
-                            headOrDefaultExpr
-                            (mkSequential
-                                mIn
-                                bodyExpr
-                                (mkSequential
-                                    mIn
-                                    (mkValSet mIn (mkLocalValRef currentVar) nextExpr)
-                                    (mkValSet mIn (mkLocalValRef nextVar) tailOrNullExpr)))
+                            (mkValSet mIn (mkLocalValRef currentVar) nextExpr)
+                            (mkValSet mIn (mkLocalValRef nextVar) tailOrNullExpr)
+
+                    let bodyAndStep =
+                        mkSequential mIn bodyExpr (Expr.DebugPoint(DebugPointAtLeafExpr.Yes range0, loopStep))
+
+                    let bodyExpr = mkLet spElem mFor elemVar headOrDefaultExpr bodyAndStep
 
                     let expr =
                         // let mutable current = enumerableExpr

src/Compiler/pars.fsy

@@ -5708,7 +5708,7 @@ arrowThenExprR:
       { let mArrow = rhs parseState 1
         let expr = $2 mArrow
         let trivia: SynExprYieldOrReturnTrivia = { YieldOrReturnKeyword = mArrow }
-        SynExpr.YieldOrReturn((true, false), expr, (unionRanges mArrow expr.Range), trivia) }
+        SynExpr.YieldOrReturn((true, false), expr, expr.Range, trivia) }
 
 forLoopBinder:
   | parenPattern IN declExpr

tests/FSharp.Compiler.ComponentTests/Debugger/CEDebugPoints.fs

@@ -30,7 +30,7 @@ let a =
     seq {
         yield 42
     }
-        """ "GenerateNext" [ (Line 6, Col 15, Line 6, Col 17) ]
+        """ "GenerateNext" [ (Line 6, Col 9, Line 6, Col 17) ]
 
     [<Fact>]
     let ``ReturnFrom in async CE - debug point covers full expression`` () =

tests/FSharp.Compiler.ComponentTests/Debugger/ForArrowDebugPoints.fs

@@ -56,7 +56,7 @@ let squares = [
             (Line 4, Col 1, Line 7, Col 6)
             (Line 5, Col 5, Line 5, Col 8)
             (Line 5, Col 11, Line 5, Col 13)
-            (Line 6, Col 15, Line 6, Col 20)
+            (Line 6, Col 9, Line 6, Col 20)
             (Line 16707566, Col 0, Line 16707566, Col 0)
         ]