Skip to content

Commit 6cfb404

Browse files
auduchinokclaude
andcommitted
Sequence points: rework for stepping
Step a for-each loop in source order: stop on the enumerable, then the element binding (covering `for <pattern>`), then the body; "getting the next element" is hidden. Add a sequence-point baseline test helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f7be8d0 commit 6cfb404

65 files changed

Lines changed: 3537 additions & 454 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

src/Compiler/Checking/Expressions/CheckExpressions.fs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8270,8 +8270,8 @@ and TcForEachExpr cenv overallTy env tpenv (seqExprOnly, isFromSource, synPat, s
82708270
let mEnumExpr = synEnumExpr.Range
82718271
let mFor = match spFor with DebugPointAtFor.Yes mStart -> mStart | DebugPointAtFor.No -> mEnumExpr
82728272
let mIn = match spIn with DebugPointAtInOrTo.Yes mStart -> mStart | DebugPointAtInOrTo.No -> mBodyExpr
8273-
let spEnumExpr = DebugPointAtBinding.Yes mEnumExpr
8274-
let spForBind = match spFor with DebugPointAtFor.Yes m -> DebugPointAtBinding.Yes m | DebugPointAtFor.No -> DebugPointAtBinding.NoneAtSticky
8273+
let spEnumExpr = match spFor with DebugPointAtFor.Yes _ -> DebugPointAtBinding.Yes mEnumExpr | DebugPointAtFor.No -> DebugPointAtBinding.NoneAtSticky
8274+
let spForBind = match spFor with DebugPointAtFor.Yes _ -> DebugPointAtBinding.Yes(unionRanges mFor mPat) | DebugPointAtFor.No -> DebugPointAtBinding.NoneAtInvisible
82758275
let spInAsWhile = match spIn with DebugPointAtInOrTo.Yes m -> DebugPointAtWhile.Yes m | DebugPointAtInOrTo.No -> DebugPointAtWhile.No
82768276

82778277
// Check the expression being enumerated
@@ -8295,10 +8295,10 @@ and TcForEachExpr cenv overallTy env tpenv (seqExprOnly, isFromSource, synPat, s
82958295
let elemTy = destArrayTy g enumExprTy
82968296

82978297
// Evaluate the array index lookup
8298-
let bodyExprFixup elemVar bodyExpr = mkInvisibleLet mIn elemVar (mkLdelem g mIn elemTy arrExpr idxExpr) bodyExpr
8298+
let bodyExprFixup elemVar bodyExpr = mkLet spForBind mFor elemVar (mkLdelem g mIn elemTy arrExpr idxExpr) bodyExpr
82998299

83008300
// Evaluate the array expression once and put it in arrVar
8301-
let overallExprFixup overallExpr = mkLet spForBind mFor arrVar enumExpr overallExpr
8301+
let overallExprFixup overallExpr = mkLet spEnumExpr mEnumExpr arrVar enumExpr overallExpr
83028302

83038303
// Ask for a loop over integers for the given range
83048304
(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
83168316
// Evaluate the span index lookup
83178317
let bodyExprFixup elemVar bodyExpr =
83188318
let elemAddrVar, _ = mkCompGenLocal mIn "addr" elemAddrTy
8319-
let e = mkInvisibleLet mIn elemVar (mkAddrGet mIn (mkLocalValRef elemAddrVar)) bodyExpr
8319+
let e = mkLet spForBind mFor elemVar (mkAddrGet mIn (mkLocalValRef elemAddrVar)) bodyExpr
83208320
let getItemCallExpr, _ = BuildMethodCall tcVal g cenv.amap PossiblyMutates mWholeExpr true getItemMethInfo ValUseFlag.NormalValUse [] [ spanExpr ] [ idxExpr ] None
83218321
mkInvisibleLet mIn elemAddrVar getItemCallExpr e
83228322

83238323
// Evaluate the span expression once and put it in spanVar
8324-
let overallExprFixup overallExpr = mkLet spForBind mFor spanVar enumExpr overallExpr
8324+
let overallExprFixup overallExpr = mkLet spEnumExpr mEnumExpr spanVar enumExpr overallExpr
83258325

83268326
let getLengthCallExpr, _ = BuildMethodCall tcVal g cenv.amap PossiblyMutates mWholeExpr true getLengthMethInfo ValUseFlag.NormalValUse [] [ spanExpr ] [] None
83278327

@@ -8410,13 +8410,13 @@ and TcForEachExpr cenv overallTy env tpenv (seqExprOnly, isFromSource, synPat, s
84108410
| Choice3Of3(enumerableVar, enumeratorVar, _, getEnumExpr, _, guardExpr, currentExpr) ->
84118411

84128412
// This compiled for must be matched EXACTLY by CompiledForEachExpr
8413-
mkLet spForBind mFor enumerableVar enumExpr
8414-
(mkLet spEnumExpr mFor enumeratorVar getEnumExpr
8413+
mkLet spEnumExpr mEnumExpr enumerableVar enumExpr
8414+
(mkInvisibleLet mFor enumeratorVar getEnumExpr
84158415
(mkTryFinally g
84168416
(mkWhile g
84178417
(spInAsWhile,
84188418
WhileLoopForCompiledForEachExprMarker, guardExpr,
8419-
mkInvisibleLet mIn elemVar currentExpr bodyExpr,
8419+
mkLet spForBind mIn elemVar currentExpr bodyExpr,
84208420
mFor),
84218421
BuildDisposableCleanup cenv env mWholeExpr enumeratorVar,
84228422
mFor, g.unit_ty, DebugPointAtTry.No, DebugPointAtFinally.No)))

src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ let TcSequenceExpression (cenv: TcFileState) env tpenv comp (overallTy: OverallT
387387

388388
Some(resultExpr, tpenv)
389389

390-
| SynExpr.YieldOrReturn(flags = (isYield, _); expr = synYieldExpr; trivia = { YieldOrReturnKeyword = m }) ->
390+
| SynExpr.YieldOrReturn(flags = (isYield, _); expr = synYieldExpr; range = m) ->
391391
let env = { env with eIsControlFlow = false }
392392
let genResultTy = NewInferenceType g
393393

@@ -404,7 +404,7 @@ let TcSequenceExpression (cenv: TcFileState) env tpenv comp (overallTy: OverallT
404404
if IsControlFlowExpression synYieldExpr then
405405
resultExpr
406406
else
407-
mkDebugPoint synYieldExpr.Range resultExpr
407+
mkDebugPoint m resultExpr
408408

409409
Some(resultExpr, tpenv)
410410

src/Compiler/CodeGen/IlxGen.fs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3172,7 +3172,11 @@ and GenExprAux (cenv: cenv) (cgbuf: CodeGenBuffer) eenv expr (sequel: sequel) =
31723172
| Expr.Match _ -> GenLinearExpr cenv cgbuf eenv expr sequel false id |> ignore<FakeUnit>
31733173

31743174
| Expr.DebugPoint(DebugPointAtLeafExpr.Yes m, innerExpr) ->
3175-
CG.EmitDebugPoint cgbuf m
3175+
if equals m range0 then
3176+
cgbuf.EmitStartOfHiddenCode()
3177+
else
3178+
CG.EmitDebugPoint cgbuf m
3179+
31763180
GenExpr cenv cgbuf eenv innerExpr sequel
31773181

31783182
| 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
37333737
Fake))
37343738

37353739
| Expr.DebugPoint(DebugPointAtLeafExpr.Yes m, innerExpr) ->
3736-
CG.EmitDebugPoint cgbuf m
3740+
if equals m range0 then
3741+
cgbuf.EmitStartOfHiddenCode()
3742+
else
3743+
CG.EmitDebugPoint cgbuf m
3744+
37373745
GenLinearExpr cenv cgbuf eenv innerExpr sequel true contf
37383746

37393747
| 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)
52805288
GenExpr cenv cgbuf eenvinner loopBody discard
52815289

52825290
// v++ or v--
5291+
cgbuf.EmitStartOfHiddenCode()
52835292
GenGetLocalVal cenv cgbuf eenvinner e2.Range v None
52845293

52855294
CG.EmitInstr cgbuf (pop 0) (Push [ g.ilg.typ_Int32 ]) (mkLdcInt32 1)

src/Compiler/Optimize/LowerComputedCollections.fs

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -339,12 +339,12 @@ module List =
339339

340340
let loop =
341341
mkLoop (fun _idxVar loopVar ->
342-
let body =
343-
body
344-
|> Option.map (fun (loopVal, body) -> mkInvisibleLet m loopVal loopVar body)
345-
|> Option.defaultValue loopVar
346-
347-
mkCallCollectorAdd tcVal g reader mBody collector body)
342+
match body with
343+
| Some (loopVal, body) ->
344+
mkInvisibleLet m loopVal loopVar
345+
(Expr.DebugPoint (DebugPointAtLeafExpr.Yes mFor, mkCallCollectorAdd tcVal g reader mBody collector body))
346+
| None ->
347+
mkCallCollectorAdd tcVal g reader mBody collector loopVar)
348348

349349
let close = mkCallCollectorClose tcVal g reader mBody collector
350350
mkSequential m loop close
@@ -504,12 +504,13 @@ module Array =
504504
mkCompGenLetIn mFor "array" arrayTy (mkNewArray count) (fun (_, array) ->
505505
let loop =
506506
mkLoop (fun idxVar loopVar ->
507-
let body =
508-
body
509-
|> Option.map (fun (loopVal, body) -> mkInvisibleLet mBody loopVal loopVar body)
510-
|> Option.defaultValue loopVar
507+
let mkStore elem = mkAsmExpr ([stelem], [], [array; convToNativeInt NoCheckOvf idxVar; elem], [], mBody)
511508

512-
mkAsmExpr ([stelem], [], [array; convToNativeInt NoCheckOvf idxVar; body], [], mBody))
509+
match body with
510+
| Some (loopVal, body) ->
511+
mkInvisibleLet mBody loopVal loopVar (Expr.DebugPoint (DebugPointAtLeafExpr.Yes mFor, mkStore body))
512+
| None ->
513+
mkStore loopVar)
513514

514515
mkSequential m loop array)
515516

src/Compiler/Optimize/Optimizer.fs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2873,11 +2873,14 @@ and OptimizeLinearExpr cenv env expr contf =
28732873
let e1R, e1info = OptimizeExpr cenv env e1
28742874

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

src/Compiler/TypedTree/TypedTreeOps.Transforms.fs

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1254,7 +1254,7 @@ module internal TupleCompilation =
12541254
Let(enumeratorVar,
12551255
GetEnumeratorCall enumerableVar2,
12561256
_enumeratorBind,
1257-
TryFinally(WhileLoopForCompiledForEachExpr(spInWhile, _, (Let(elemVar, _, _, bodyExpr) as elemLet), _), _))) when
1257+
TryFinally(WhileLoopForCompiledForEachExpr(spInWhile, _, (Let(elemVar, _, spElem, bodyExpr) as elemLet), _), _))) when
12581258
// Apply correctness conditions to ensure this really is a compiled for-each expression.
12591259
valRefEq g (mkLocalValRef enumerableVar) enumerableVar2
12601260
&& enumerableVar.IsCompilerGenerated
@@ -1271,7 +1271,7 @@ module internal TupleCompilation =
12711271
let mIn = elemLet.Range
12721272

12731273
let mFor =
1274-
match spFor with
1274+
match spElem with
12751275
| DebugPointAtBinding.Yes mFor -> mFor
12761276
| _ -> enumerableExpr.Range
12771277

@@ -1287,7 +1287,7 @@ module internal TupleCompilation =
12871287

12881288
let enumerableTy = tyOfExpr g enumerableExpr
12891289

1290-
ValueSome(enumerableTy, enumerableExpr, elemVar, bodyExpr, (mBody, spFor, spIn, mFor, mIn, spInWhile, mWholeExpr))
1290+
ValueSome(enumerableTy, enumerableExpr, elemVar, bodyExpr, (mBody, spFor, spElem, spIn, mFor, mIn, spInWhile, mWholeExpr))
12911291
| _ -> ValueNone
12921292

12931293
[<return: Struct>]
@@ -2147,7 +2147,7 @@ module internal TupleCompilation =
21472147
match option, expr with
21482148
| _, CompiledInt32RangeForEachExpr g (startExpr, (1 | -1 as step), finishExpr, elemVar, bodyExpr, ranges) ->
21492149

2150-
let _mBody, spFor, spIn, _mFor, _mIn, _spInWhile, mWholeExpr = ranges
2150+
let _mBody, spFor, _spElem, spIn, _mFor, _mIn, _spInWhile, mWholeExpr = ranges
21512151

21522152
let spFor =
21532153
match spFor with
@@ -2164,13 +2164,13 @@ module internal TupleCompilation =
21642164
ValueNone)
21652165
with
21662166
| ValueSome(rangeTy, (start, step, finish)) ->
2167-
let mBody, _spFor, _spIn, mFor, mIn, spInWhile, _mWhole = ranges
2167+
let mBody, _spFor, _spElem, _spIn, mFor, mIn, spInWhile, _mWhole = ranges
21682168

21692169
mkOptimizedRangeLoop g (mBody, mFor, mIn, spInWhile) (rangeTy, enumerableExpr) (start, step, finish) (fun _count mkLoop ->
21702170
mkLoop (fun _idxVar loopVar -> mkInvisibleLet elemVar.Range elemVar loopVar bodyExpr))
21712171
| ValueNone ->
21722172

2173-
let mBody, spFor, spIn, mFor, mIn, spInWhile, mWholeExpr = ranges
2173+
let mBody, spFor, spElem, spIn, mFor, mIn, spInWhile, mWholeExpr = ranges
21742174

21752175
if isStringTy g enumerableTy then
21762176
// type is string, optimize for expression as:
@@ -2189,7 +2189,7 @@ module internal TupleCompilation =
21892189
let finishExpr = mkDecr g mFor lengthExpr
21902190
// for compat reasons, loop item over string is sometimes object, not char
21912191
let loopItemExpr = mkCoerceIfNeeded g elemVar.Type g.char_ty charExpr
2192-
let bodyExpr = mkInvisibleLet mIn elemVar loopItemExpr bodyExpr
2192+
let bodyExpr = mkLet spElem mFor elemVar loopItemExpr bodyExpr
21932193

21942194
let forExpr =
21952195
mkFastForLoop g (DebugPointAtFor.No, spIn, mWholeExpr, idxVar, startExpr, true, finishExpr, bodyExpr)
@@ -2223,18 +2223,16 @@ module internal TupleCompilation =
22232223
let tailOrNullExpr =
22242224
mkUnionCaseFieldGetUnprovenViaExprAddr (currentExpr, g.cons_ucref, [ elemTy ], IndexTail, mIn)
22252225

2226-
let bodyExpr =
2227-
mkInvisibleLet
2226+
let loopStep =
2227+
mkSequential
22282228
mIn
2229-
elemVar
2230-
headOrDefaultExpr
2231-
(mkSequential
2232-
mIn
2233-
bodyExpr
2234-
(mkSequential
2235-
mIn
2236-
(mkValSet mIn (mkLocalValRef currentVar) nextExpr)
2237-
(mkValSet mIn (mkLocalValRef nextVar) tailOrNullExpr)))
2229+
(mkValSet mIn (mkLocalValRef currentVar) nextExpr)
2230+
(mkValSet mIn (mkLocalValRef nextVar) tailOrNullExpr)
2231+
2232+
let bodyAndStep =
2233+
mkSequential mIn bodyExpr (Expr.DebugPoint(DebugPointAtLeafExpr.Yes range0, loopStep))
2234+
2235+
let bodyExpr = mkLet spElem mFor elemVar headOrDefaultExpr bodyAndStep
22382236

22392237
let expr =
22402238
// let mutable current = enumerableExpr

src/Compiler/pars.fsy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5708,7 +5708,7 @@ arrowThenExprR:
57085708
{ let mArrow = rhs parseState 1
57095709
let expr = $2 mArrow
57105710
let trivia: SynExprYieldOrReturnTrivia = { YieldOrReturnKeyword = mArrow }
5711-
SynExpr.YieldOrReturn((true, false), expr, (unionRanges mArrow expr.Range), trivia) }
5711+
SynExpr.YieldOrReturn((true, false), expr, expr.Range, trivia) }
57125712

57135713
forLoopBinder:
57145714
| parenPattern IN declExpr

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ let a =
3030
seq {
3131
yield 42
3232
}
33-
""" "GenerateNext" [ (Line 6, Col 15, Line 6, Col 17) ]
33+
""" "GenerateNext" [ (Line 6, Col 9, Line 6, Col 17) ]
3434

3535
[<Fact>]
3636
let ``ReturnFrom in async CE - debug point covers full expression`` () =

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ let squares = [
5656
(Line 4, Col 1, Line 7, Col 6)
5757
(Line 5, Col 5, Line 5, Col 8)
5858
(Line 5, Col 11, Line 5, Col 13)
59-
(Line 6, Col 15, Line 6, Col 20)
59+
(Line 6, Col 9, Line 6, Col 20)
6060
(Line 16707566, Col 0, Line 16707566, Col 0)
6161
]
6262

0 commit comments

Comments
 (0)