Debug: rework conditional erasure, fix stepping over literals (#19897)

Author: auduchinok

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

@@ -92,6 +92,7 @@
 * FCS: add FSharpCheckFileResults.HasErrors ([PR #19892](https://github.com/dotnet/fsharp/pull/19892))
 * 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))
+* Debug: rework conditional erasure, fix stepping over literals ([PR #19897](https://github.com/dotnet/fsharp/pull/19897))
 
 ### Changed
 

src/Compiler/Checking/CheckIncrementalClasses.fs

@@ -802,7 +802,7 @@ let MakeCtorForIncrClassConstructionPhase2C(
                 // Extend the range of any immediate debug point to include the 'do'
                 let doExpr =
                     match doExpr with
-                    | Expr.DebugPoint(_, innerExpr) -> Expr.DebugPoint(DebugPointAtLeafExpr.Yes mFull, innerExpr)
+                    | Expr.DebugPoint(_, innerExpr) -> Expr.DebugPoint(DebugPointAtLeafExpr.Yes(false, mFull), innerExpr)
                     | e -> e
                 let binder = (fun e -> mkSequential mFull doExpr e)
                 let isPriorToSuperInit = false
@@ -940,7 +940,7 @@ let MakeCtorForIncrClassConstructionPhase2C(
             // Add the debug point
             let inheritsExpr =
                 if inheritsIsVisible then
-                    Expr.DebugPoint(DebugPointAtLeafExpr.Yes inheritsExpr.Range, inheritsExpr)
+                    Expr.DebugPoint(DebugPointAtLeafExpr.Yes(false, inheritsExpr.Range), inheritsExpr)
                 else
                     inheritsExpr
 

src/Compiler/Checking/Expressions/CheckComputationExpressions.fs

@@ -81,7 +81,7 @@ let inline arbKeySelectors m =
 // Flag that a debug point should get emitted prior to both the evaluation of 'rhsExpr' and the call to Using
 let inline addBindDebugPoint spBind e =
     match spBind with
-    | DebugPointAtBinding.Yes m -> SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes m, false, e)
+    | DebugPointAtBinding.Yes m -> SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes(false, m), false, e)
     | _ -> e
 
 let inline mkSynDelay2 (e: SynExpr) = mkSynDelay (e.Range.MakeSynthetic()) e
@@ -1345,7 +1345,7 @@ let rec TryTranslateComputationExpression
 
                     let forCall =
                         match spFor with
-                        | DebugPointAtFor.Yes _ -> SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes mFor, false, forCall)
+                        | DebugPointAtFor.Yes _ -> SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes(false, mFor), false, forCall)
                         | DebugPointAtFor.No -> forCall
 
                     translatedCtxt forCall)
@@ -1389,7 +1389,7 @@ let rec TryTranslateComputationExpression
             // 'while' is hit just before each time the guard is called
             let guardExpr =
                 match spWhile with
-                | DebugPointAtWhile.Yes _ -> SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes mWhile, false, guardExpr)
+                | DebugPointAtWhile.Yes _ -> SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes(false, mWhile), false, guardExpr)
                 | DebugPointAtWhile.No -> guardExpr
 
             Some(
@@ -1419,7 +1419,7 @@ let rec TryTranslateComputationExpression
             // 'while!' is hit just before each time the guard is called
             let guardExpr =
                 match spWhile with
-                | DebugPointAtWhile.Yes _ -> SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes mWhile, false, guardExpr)
+                | DebugPointAtWhile.Yes _ -> SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes(false, mWhile), false, guardExpr)
                 | DebugPointAtWhile.No -> guardExpr
 
             let rewrittenWhileExpr =
@@ -1557,7 +1557,7 @@ let rec TryTranslateComputationExpression
             // Put down a debug point for the 'finally'
             let unwindExpr2 =
                 match spFinally with
-                | DebugPointAtFinally.Yes _ -> SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes mFinally, true, unwindExpr)
+                | DebugPointAtFinally.Yes _ -> SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes(false, mFinally), true, unwindExpr)
                 | DebugPointAtFinally.No -> unwindExpr
 
             if ceenv.isQuery then
@@ -1571,7 +1571,7 @@ let rec TryTranslateComputationExpression
 
             let innerExpr =
                 match spTry with
-                | DebugPointAtTry.Yes _ -> SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes mTry, true, innerExpr)
+                | DebugPointAtTry.Yes _ -> SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes(false, mTry), true, innerExpr)
                 | _ -> innerExpr
 
             Some(
@@ -2308,7 +2308,7 @@ let rec TryTranslateComputationExpression
 
             let innerExpr =
                 match spTry with
-                | DebugPointAtTry.Yes _ -> SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes mTry, true, innerExpr)
+                | DebugPointAtTry.Yes _ -> SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes(false, mTry), true, innerExpr)
                 | _ -> innerExpr
 
             let callExpr =
@@ -2345,7 +2345,7 @@ let rec TryTranslateComputationExpression
                 if IsControlFlowExpression synYieldExpr then
                     yieldFromCall
                 else
-                    SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes mFull, false, yieldFromCall)
+                    SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes(false, mFull), false, yieldFromCall)
 
             Some(translatedCtxt yieldFromCall)
 
@@ -2374,7 +2374,7 @@ let rec TryTranslateComputationExpression
                 if IsControlFlowExpression synReturnExpr then
                     returnFromCall
                 else
-                    SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes mFull, false, returnFromCall)
+                    SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes(false, mFull), false, returnFromCall)
 
             Some(translatedCtxt returnFromCall)
 
@@ -2393,7 +2393,7 @@ let rec TryTranslateComputationExpression
                 if IsControlFlowExpression synYieldOrReturnExpr then
                     yieldOrReturnCall
                 else
-                    SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes mFull, false, yieldOrReturnCall)
+                    SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes(false, mFull), false, yieldOrReturnCall)
 
             Some(translatedCtxt yieldOrReturnCall)
 
@@ -2704,7 +2704,9 @@ and TranslateComputationExpressionBind
 and convertSimpleReturnToExpr (ceenv: ComputationExpressionContext<'a>) comp varSpace innerComp =
     match innerComp with
     | SynExpr.YieldOrReturn((false, _), returnExpr, m, _) ->
-        let returnExpr = SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes m, false, returnExpr)
+        let returnExpr =
+            SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes(false, m), false, returnExpr)
+
         Some(returnExpr, None)
 
     | SynExpr.Match(spMatch, expr, clauses, m, trivia) ->

src/Compiler/Checking/Expressions/CheckExpressions.fs

@@ -3103,7 +3103,7 @@ let BuildPossiblyConditionalMethodCall (cenv: cenv) env isMutable m isProp minfo
     if shouldEraseCall then
         // Methods marked with 'Conditional' must return 'unit'
         UnifyTypes cenv env m g.unit_ty (minfo.GetFSharpReturnType(cenv.amap, m, minst))
-        mkUnit g m, g.unit_ty
+        Expr.DebugPoint(DebugPointAtLeafExpr.Yes(isHidden = true, range = m), mkUnit g m), g.unit_ty
     else
 #if !NO_TYPEPROVIDERS
         match minfo with
@@ -5903,6 +5903,7 @@ and TcNonControlFlowExpr (env: TcEnv) f =
             let res2 =
                 match res with
                 | IfThenElseExpr _ -> res
+                | Expr.DebugPoint(DebugPointAtLeafExpr.Yes(isHidden = true), _) -> res
                 | _ -> mkDebugPoint res.Range res
             res2, tpenv
     else

src/Compiler/CodeGen/IlxGen.fs

@@ -3174,8 +3174,8 @@ and GenExprAux (cenv: cenv) (cgbuf: CodeGenBuffer) eenv expr (sequel: sequel) =
         | LinearOpExpr _
         | Expr.Match _ -> GenLinearExpr cenv cgbuf eenv expr sequel false id |> ignore<FakeUnit>
 
-        | Expr.DebugPoint(DebugPointAtLeafExpr.Yes m, innerExpr) ->
-            if Range.equals m range0 then
+        | Expr.DebugPoint(DebugPointAtLeafExpr.Yes(isHidden, m), innerExpr) ->
+            if isHidden then
                 cgbuf.EmitStartOfHiddenCode()
             else
                 CG.EmitDebugPoint cgbuf m
@@ -3739,8 +3739,8 @@ and GenLinearExpr cenv cgbuf eenv expr sequel preSteps (contf: FakeUnit -> FakeU
                          GenSequel cenv eenv.cloc cgbuf sequelAfterJoin
                          Fake))
 
-    | Expr.DebugPoint(DebugPointAtLeafExpr.Yes m, innerExpr) ->
-        if Range.equals m range0 then
+    | Expr.DebugPoint(DebugPointAtLeafExpr.Yes(isHidden, m), innerExpr) ->
+        if isHidden then
             cgbuf.EmitStartOfHiddenCode()
         else
             CG.EmitDebugPoint cgbuf m
@@ -10659,10 +10659,21 @@ and CodeGenInitMethod cenv (cgbuf: CodeGenBuffer) eenv tref (codeGenInitFunc: Co
     let _, body =
         CodeGenMethod cenv cgbuf.mgbuf ([], eenv.staticInitializationName, eenv, 0, None, codeGenInitFunc, m)
 
-    if CheckCodeDoesSomething body.Code then
+    let codeDoesSomething = CheckCodeDoesSomething body.Code
+
+    // Keep the init method if it carries a visible debug point, so steppable bindings like 'let i = ()' survive.
+    let hasVisibleDebugPoint =
+        not cenv.options.localOptimizationsEnabled
+        && body.Code.Instrs
+           |> Array.exists (function
+               | I_seqpoint sp -> sp.Line <> FeeFee cenv
+               | _ -> false)
+
+    if codeDoesSomething || hasVisibleDebugPoint then
         // We are here because the module we just grabbed has an interesting static initializer
         let feefee, seqpt =
-            if body.Code.Instrs.Length > 0 then
+            // Without real init code, the .cctor's FeeFee marker would just add a stray hidden sequence point.
+            if codeDoesSomething && body.Code.Instrs.Length > 0 then
                 match body.Code.Instrs[0] with
                 | I_seqpoint sp as i -> [ FeeFeeInstr cenv sp.Document ], [ i ]
                 | _ -> [], []

src/Compiler/Optimize/LowerComputedCollections.fs

@@ -114,7 +114,7 @@ let LowerComputedListOrArraySeqExpr tcVal g amap m collectorTy overallSeqExpr =
                 let cleanupE = BuildDisposableCleanup tcVal g infoReader m enumv
 
                 // A debug point should get emitted prior to both the evaluation of 'inp' and the call to GetEnumerator
-                let addForDebugPoint e = Expr.DebugPoint(DebugPointAtLeafExpr.Yes mFor, e)
+                let addForDebugPoint e = Expr.DebugPoint(DebugPointAtLeafExpr.Yes(false, mFor), e)
 
                 let spInAsWhile = match spIn with DebugPointAtInOrTo.Yes m -> DebugPointAtWhile.Yes m | DebugPointAtInOrTo.No -> DebugPointAtWhile.No
 
@@ -289,7 +289,7 @@ module List =
                     match body with
                     | Expr.Let(TBind(v, rhs, DebugPointAtBinding.Yes spBind), innerBody, m, flags) ->
                         let bodyForAdd = Expr.Let(TBind(v, rhs, DebugPointAtBinding.NoneAtInvisible), innerBody, m, flags)
-                        Expr.DebugPoint(DebugPointAtLeafExpr.Yes spBind, mkCallCollectorAdd tcVal g reader mBody collector bodyForAdd)
+                        Expr.DebugPoint(DebugPointAtLeafExpr.Yes(false, spBind), mkCallCollectorAdd tcVal g reader mBody collector bodyForAdd)
                     | _ ->
                         mkCallCollectorAdd tcVal g reader mIn collector body
 
@@ -342,7 +342,7 @@ module List =
                         match body with
                         | Some (loopVal, body) ->
                             mkInvisibleLet m loopVal loopVar
-                                (Expr.DebugPoint (DebugPointAtLeafExpr.Yes mFor, mkCallCollectorAdd tcVal g reader mBody collector body))
+                                (Expr.DebugPoint (DebugPointAtLeafExpr.Yes(false, mFor), mkCallCollectorAdd tcVal g reader mBody collector body))
                         | None ->
                             mkCallCollectorAdd tcVal g reader mBody collector loopVar)
 
@@ -447,7 +447,7 @@ module Array =
                 )
 
             // Add a debug point at the `for`, before anything gets evaluated.
-            Expr.DebugPoint (DebugPointAtLeafExpr.Yes mFor, mapping)
+            Expr.DebugPoint (DebugPointAtLeafExpr.Yes(false, mFor), mapping)
         )
 
     /// Whether to check for overflow when converting a value to a native int.
@@ -508,7 +508,7 @@ module Array =
 
                         match body with
                         | Some (loopVal, body) ->
-                            mkInvisibleLet mBody loopVal loopVar (Expr.DebugPoint (DebugPointAtLeafExpr.Yes mFor, mkStore body))
+                            mkInvisibleLet mBody loopVal loopVar (Expr.DebugPoint (DebugPointAtLeafExpr.Yes(false, mFor), mkStore body))
                         | None ->
                             mkStore loopVar)
 

src/Compiler/Optimize/LowerSequences.fs

@@ -118,7 +118,7 @@ let ConvertSequenceExprToObject g amap overallExpr =
         let (TBind(v, e, sp)) = bind
         let addDebugPoint e =
             match sp with
-            | DebugPointAtBinding.Yes m -> Expr.DebugPoint(DebugPointAtLeafExpr.Yes m, e)
+            | DebugPointAtBinding.Yes m -> Expr.DebugPoint(DebugPointAtLeafExpr.Yes(false, m), e)
             | _ -> e
         let vref = mkLocalValRef v
         { resBody with
@@ -262,7 +262,7 @@ let ConvertSequenceExprToObject g amap overallExpr =
             //       body ]]
 
             // A debug point should get emitted prior to both the evaluation of 'inp' and the call to GetEnumerator
-            let addForDebugPoint e = Expr.DebugPoint(DebugPointAtLeafExpr.Yes mFor, e)
+            let addForDebugPoint e = Expr.DebugPoint(DebugPointAtLeafExpr.Yes(false, mFor), e)
 
             // The 'in' debug point is put back into the TypedTree at the right place for SeqWhile
             let mIn = match spIn with DebugPointAtInOrTo.Yes m -> m.NoteSourceConstruct(NotedSourceConstruct.While) | DebugPointAtInOrTo.No -> mIn
@@ -293,11 +293,11 @@ let ConvertSequenceExprToObject g amap overallExpr =
                 let asyncVars = unionFreeVars res1.asyncVars (freeInExpr CollectLocals compensation)
                 let addTryDebugPoint e =
                     match spTry with
-                    | DebugPointAtTry.Yes m -> Expr.DebugPoint(DebugPointAtLeafExpr.Yes m, e)
+                    | DebugPointAtTry.Yes m -> Expr.DebugPoint(DebugPointAtLeafExpr.Yes(false, m), e)
                     | _ -> e
                 let addFinallyDebugPoint e =
                     match spFinally with
-                    | DebugPointAtFinally.Yes m -> Expr.DebugPoint(DebugPointAtLeafExpr.Yes m, e)
+                    | DebugPointAtFinally.Yes m -> Expr.DebugPoint(DebugPointAtLeafExpr.Yes(false, m), e)
                     | _ -> e
                 Some { phase2 = (fun (pcVar, _currv, _, pcMap as ctxt) ->
                             let generate1, dispose1, checkDispose1 = res1.phase2 ctxt

src/Compiler/Optimize/LowerStateMachines.fs

@@ -76,7 +76,7 @@ let RepresentBindingAsStateVar g (bind: Binding) (resBody: StateMachineConversio
     let (TBind(v, e, sp)) = bind
     let addDebugPoint innerExpr =
         match sp with
-        | DebugPointAtBinding.Yes m -> Expr.DebugPoint(DebugPointAtLeafExpr.Yes m, innerExpr)
+        | DebugPointAtBinding.Yes m -> Expr.DebugPoint(DebugPointAtLeafExpr.Yes(false, m), innerExpr)
         | _ -> innerExpr
     let vref = mkLocalValRef v
     { resBody with

src/Compiler/Optimize/Optimizer.fs

@@ -383,9 +383,6 @@ type OptimizationSettings =
     /// This optimization is off by default, given tiny overhead of including try/with. See https://github.com/dotnet/fsharp/pull/376
     member _.EliminateTryWithAndTryFinally = false 
 
-    /// Determines if we should eliminate first part of sequential expression if it has no effect 
-    member x.EliminateSequential = x.LocalOptimizationsEnabled 
-
     /// Determines if we should determine branches in pattern matching based on known information, e.g.
     /// eliminate a "if true then .. else ... "
     member x.EliminateSwitch = x.LocalOptimizationsEnabled
@@ -2896,12 +2893,8 @@ and OptimizeLinearExpr cenv env expr contf =
 
       OptimizeLinearExpr cenv env e2 (contf << (fun (e2R, e2info) -> 
         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)) &&
+           (cenv.settings.LocalOptimizationsEnabled ||
+            (match e1R with | Expr.DebugPoint(DebugPointAtLeafExpr.Yes(isHidden, _), _) -> isHidden | _ -> false)) &&
            not e1info.HasEffect then
             e2R, e2info
         else 
@@ -2964,9 +2957,11 @@ and OptimizeLinearExpr cenv env expr contf =
          OptimizeLinearExpr cenv env argLast (contf << (fun (argLastR, argLastInfo) ->
              OptimizeExprOpReductionsAfter cenv env (op, tyargs, argsHeadR @ [argLastR], argsHeadInfosR @ [argLastInfo], m)))
 
-    | Expr.DebugPoint (m, innerExpr) when not (IsDebugPipeRightExpr cenv innerExpr)-> 
+    | Expr.DebugPoint (m, innerExpr) when not (IsDebugPipeRightExpr cenv innerExpr)->
         OptimizeLinearExpr cenv env innerExpr (contf << (fun (innerExprR, einfo) ->
-            Expr.DebugPoint (m, innerExprR), einfo))
+            match m with
+            | DebugPointAtLeafExpr.Yes(isHidden = true) when not einfo.HasEffect -> innerExprR, einfo
+            | _ -> Expr.DebugPoint (m, innerExprR), einfo))
 
     | _ -> contf (OptimizeExpr cenv env expr)
 
@@ -2988,7 +2983,7 @@ and OptimizeTryFinally cenv env (spTry, spFinally, e1, e2, m, ty) =
     if cenv.settings.EliminateTryWithAndTryFinally && not e1info.HasEffect then 
         let e1R2 = 
             match spTry with 
-            | DebugPointAtTry.Yes m -> Expr.DebugPoint(DebugPointAtLeafExpr.Yes m, e1R)
+            | DebugPointAtTry.Yes m -> Expr.DebugPoint(DebugPointAtLeafExpr.Yes(false, m), e1R)
             | DebugPointAtTry.No -> e1R
         Expr.Sequential (e1R2, e2R, ThenDoSeq, m), info 
     else
@@ -4242,7 +4237,14 @@ and OptimizeBinding cenv isRec env (TBind(vref, expr, spBind)) =
         if vref.ShouldInline && IsPartialExprVal einfo.Info then 
             errorR(InternalError("the inline value '"+vref.LogicalName+"' was not inferred to have a known value", vref.Range))
 
-        let env = BindInternalLocalVal cenv vref (mkValInfo einfo vref) env 
+        let env = BindInternalLocalVal cenv vref (mkValInfo einfo vref) env
+
+        // The hidden debug point on the r.h.s. is dropped above, so suppress the binding's debug point too.
+        let spBind =
+            match expr with
+            | Expr.DebugPoint(DebugPointAtLeafExpr.Yes(isHidden = true), _) when not einfo.HasEffect -> DebugPointAtBinding.NoneAtLet
+            | _ -> spBind
+
         (TBind(vref, exprOptimized, spBind), einfo), env
     with RecoverableException exn -> 
         errorRecovery exn vref.Range 

src/Compiler/Service/FSharpParseFileResults.fs

@@ -568,7 +568,7 @@ type FSharpParseFileResults(diagnostics: FSharpDiagnostic[], input: ParsedInput,
                                             | SynInterpolatedStringPart.FillExpr(fillExpr, _) -> yield fillExpr
                                     ]
 
-                        | SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes m, isControlFlow, innerExpr) ->
+                        | SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes(_, m), isControlFlow, innerExpr) ->
                             yield! checkRange m
                             yield! walkExpr isControlFlow innerExpr