Debug: rework for expressions stepping#19894
Merged
Merged
Conversation
Contributor
❗ Release notes required
|
auduchinok
changed the title
for expressions stepping
auduchinok
changed the title
for expressions steppingfor expressions stepping
Member
Author
|
@majocha May I ask you to check if there're any regressions in stepping in this PR, please? You've caught a very good one in the inlining PR. I expect many cases to be improved, but I could've missed something. |
github-actions
Bot
added
the
⚠️ Affects-Compiler-Output
This comment has been minimized.
This comment has been minimized.
Contributor
Sure! It would also be great to put some new samples in TheBigFileOfDebugStepping.fsx to do some before / after, especially the "for with active pattern" case. |
Member
Author
I'm currently adding these cases as sequence point dumps, like this one. |
github-actions
Bot
added
the
⚠️ Affects-Test-Tooling
This comment has been minimized.
This comment has been minimized.
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>
auduchinok
force-pushed
the
debug-sequencePoint-for
branch
from
54598b1 to
29cd63c
Compare
Contributor
|
🔍 Tooling Safety Check — Affects-Bootstrap, Affects-Compiler-Output
|
github-actions
Bot
added
the
⚠️ Affects-Bootstrap
Member
Author
|
@T-Gro Could the bot comments update the first comment, like it's done for the release notes, instead of posting more replies, please? |
T-Gro
approved these changes
Jun 8, 2026
github-project-automation
Bot
moved this from New
to In Progress
in F# Compiler and Tooling
github-project-automation
Bot
moved this from In Progress
to Done
in F# Compiler and Tooling
T-Gro
added a commit
that referenced
this pull request
Jun 8, 2026
Refute and dismiss B1 ("debug-stepping clobber"): PR #19894 (d0e593f)
landed on origin/main on 2026-06-08 13:11, three days AFTER my last merge
on 2026-06-05 11:35. None of my commits (7757717, e2ed10f, f933f83)
touch the relevant IlxGen.fs lines. Merged origin/main now brings the
fix in — verified `if equals m range0` is present at IlxGen.fs:3178 and
:3743 in the working tree.
Apply M3: Release notes now cite #19075 in the constraint-stripping entry
(test `SRTP member constraint with IDisposable` explicitly targets that
issue's CLR segfault).
Soften M2: Drop the unverified "≈23x" specific number from release notes;
the perf magnitude is documented in #17607 itself with the original repro.
Apply L2: `unmanaged + equality` test now asserts no
`Specialize<valuetype (...modreq...)` or `T<valuetype (...modreq...)`
leakage — exercises the IsUnmanagedAttribute / modreq stripping path that
motivated the CustomAttrsStored clear in stripILGenericParamConstraints.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
T-Gro
added a commit
that referenced
this pull request
Jun 9, 2026
ILVerify on this PR (build 1454404) hit the 60-min default timeout and was cancelled mid-Release-build. Investigation against recent main builds: Build 1449768 (pre-#19894) : ILVerify 21.1 min, succeeded Build 1451079 (pre-#19894) : ILVerify 21.2 min, succeeded Build 1453983 (#19894 `d0e593f67`) : ILVerify 36.5 min, succeeded Build 1454404 (this PR HEAD): ILVerify 60.4 min, CANCELLED Two compounding causes: 1. Upstream PR #19894 ("Debug: rework or expressions stepping") added ~14 min to ILVerify on main alone — verified across multiple post-#19894 main builds. ILVerify is the only leg using -bootstrap, so it pays the compiler-build cost x3 (bootstrap, proto, final). 2. This PR makes TLR actually fire under --realsig+ for the first time (the whole point of #17607's fix). The compiler self-build now lifts thousands of inner-rec functions across FCS itself that were previously left as closures. That extra TLR work, multiplied by the x3 bootstrap cycle, pushes ILVerify past 60 min. Neither is "flaky". The work is real and load-bearing. Bumping the per-job timeout to 120 min restores headroom for the bootstrap+proto+final cycle across both Debug and Release configurations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes various stepping issues in
forexpressions:Covers list, array, seq cases and fixes:
forsequence point now covers the pattern, so it's possible to step into the active pattern in the rangeforsequence point covering the whole comprehensionTo make the changes observable this PR also introduces a new test helper dumping sequence points along with their range and instructions.