-
Notifications
You must be signed in to change notification settings - Fork 849
Bugfix :: Sequence points #19278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
T-Gro
wants to merge
18
commits into
main
Choose a base branch
from
bugfix/sequence-points
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Bugfix :: Sequence points #19278
+5,346
−5,074
Conversation
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
Breakpoints inside 'for x in xs -> body' (arrow syntax in list/array/seq comprehensions) failed to bind because the SeqMap active pattern in LowerComputedCollections.fs did not preserve debug point wrappers. Fix: wrap body extraction in DebugPoints and return 'debug body', matching the existing SeqCollectSingle pattern. Add 3 tests verifying debug points exist on body expressions for list arrow, array arrow, and do-yield comprehension forms.
range (e.g., 'return 1' not just 'return') by using mFull instead of keyword-only range m in DebugPointAtLeafExpr.Yes - Fix use binding in CE to use DebugPointAtTarget.No on the SynMatchClause to avoid generating an extra sequence point from the match target - Add VerifyMethodSequencePoints test infrastructure to verify sequence points for specific methods (e.g., Invoke, MoveNext, GenerateNext) - Add CEDebugPoints test module with 3 tests validating the fixes
- Fix compiler: use full binding range (mBind) instead of keyword-only range (leadingKeyword.Range) in mkSynCall for 'use' CE translation, eliminating the use-keyword-only sequence point in state machine output. - Extract verifyCEMethodDebugPoints helper to reduce test boilerplate. - Change use test from async/Invoke to task/MoveNext for single-method SP validation in the state machine. - Update expected SP values to match actual compiler output. - All 3 CEDebugPoints tests pass; PortablePdbs, ForArrowDebugPoints, and AsyncExpressionStepping regression tests pass.
Contributor
❗ Release notes required
Warning No PR link found in some release notes, please consider adding it.
|
…harp into bugfix/sequence-points
38c2f6e to
1ce8e10
Compare
Member
Author
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
Member
Author
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
Member
Author
|
/azp run fsharp-ci |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
Member
Author
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Debug point / sequence point fixes
Fixes Extra sequence point at the end of match expressions #12052 — Extra sequence point at end of
matchexpressions caused debugger to briefly stop at unrelated branches during Step Over. Fixed by always emitting hidden code at decision tree join points inIlxGen.fs, not only when the stack is non-empty. Also covers theif/elif/elsecase mentioned in the comments. Tests:MatchEndSequencePoint.fs(5 tests including if/then/else).Fixes Wrong sequence point range for
returninsideasynccomputation expression #19248 —return,yield,return!,yield!in CEs had debug points covering only the keyword range instead of the full expression (e.g.returninstead ofreturn 1). Fixed by using the full expression range (mFull) forDebugPointAtLeafExprinCheckComputationExpressions.fs. Tests:CEDebugPoints.fs(6 tests covering return, yield, return!, yield! in async/task/custom CEs).Fixes Extra out-of-order sequence point for
useintaskcomputation expressions #19255 —usebindings intaskCEs emitted an extra out-of-order sequence point (nop at theuseline beforeleton the prior line), making execution appear to jump backwards. Fixed by usingmBindinstead ofleadingKeyword.Rangefor theUsingcall and switchingDebugPointAtTargettoNo. Tests:CEDebugPoints.fs(Use in task CEtest).Fixes Debug points failing to bind in body of "[ for x in xs -> body ]" #13504 — Debug points failed to bind in
[ for x in xs -> body ]arrow comprehensions. TheSeqMappattern inLowerComputedCollections.fswas stripping debug points from the loop body during lowering. Fixed by preserving them viaDebugPointsactive pattern. Tests:ForArrowDebugPoints.fs(3 tests: list, array, and for-do-yield contrast).