Redo the InfiniteSeq module to prefer exceptions to Results#101
Merged
Redo the InfiniteSeq module to prefer exceptions to Results#101
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reworks the InfiniteSeq API to treat “hang” scenarios as exceptions (rather than Result/Option), and introduces a reusable Seq.isHungAfter guard to cap evaluation of potentially infinite sequences across the codebase.
Changes:
- Added
InfiniteSequenceEvaluationHungexception andSeq.isHungAfter(plusNonEmptywrapper) to detect/stop runaway sequence evaluation. - Updated
InfiniteSeqto be directly iterable (IEnumerable) and added new exception-throwing APIs (item,take,takeWhile,head,uncons,find, etc.), deprecating the previousResult/Option-returning APIs. - Updated docs and specs to cover the new APIs and exception behavior.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| SafetyFirst/Seq.fs | Adds Seq.isHungAfter; rewrites skipLenient to avoid double enumeration; adds NonEmpty.isHungAfter. |
| SafetyFirst/InfiniteSeq.fs | Refactors InfiniteSeq to be enumerable, adds exception-first APIs, and marks older Result/Option APIs obsolete. |
| SafetyFirst/ErrorTypes.fs | Introduces InfiniteSequenceEvaluationHung exception type. |
| SafetyFirst.Specs/InfiniteSeqSpec.fs | Adds/updates tests for new InfiniteSeq APIs and exception behavior. |
| ReleaseNotes.md | Documents the InfiniteSeq exception-first redesign and new APIs. |
| README.md | Updates InfiniteSeq documentation and references Seq.isHungAfter. |
| .gitignore | Ignores .claude/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+3
to
+4
| #nowarn "44" | ||
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
Reworks the InfiniteSeq API to prefer throwing InfiniteSequenceEvaluationHung over returning Result/Option on hang detection, aligning infinite-sequence “hang” with a bug/crash model rather than a recoverable error.
Changes:
- Adds
Seq.isHungAfter(andSeq.NonEmpty.isHungAfter) as the primary hang-guard mechanism via exceptions. - Refactors
InfiniteSeqto be directly iterable (IEnumerable) and introduces new APIs (initBounded,initUnbounded,assume,append, plus non-Resultversions ofitem/take/head/etc.), while marking legacyResult/OptionAPIs obsolete. - Updates specs and docs (ReleaseNotes/README) to reflect the new exception-first approach.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| SafetyFirst/Seq.fs | Adds Seq.isHungAfter; rewrites skipLenient; adds NonEmpty.isHungAfter. |
| SafetyFirst/InfiniteSeq.fs | Makes InfiniteSeq enumerable; shifts core APIs to throw-on-hang; keeps legacy APIs as [<Obsolete>]. |
| SafetyFirst/ErrorTypes.fs | Introduces InfiniteSequenceEvaluationHung : Exception. |
| SafetyFirst.Specs/SeqSpec.fs | Adds unit tests for Seq.isHungAfter. |
| SafetyFirst.Specs/InfiniteSeqSpec.fs | Adds tests for new InfiniteSeq APIs and exception behavior; suppresses obsolete warnings. |
| ReleaseNotes.md | Documents v5.4.0 behavior change and new APIs. |
| README.md | Updates user-facing guidance for InfiniteSeq and Seq.isHungAfter. |
| .gitignore | Ignores .claude/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+49
to
+50
| iter.MoveNext() |> ignore | ||
| iter.Current) () |
Comment on lines
+463
to
+475
| seq { | ||
| use e = xs.GetEnumerator() | ||
| let mutable remaining = count | ||
| let mutable enoughElements = true | ||
| while remaining > 0 && enoughElements do | ||
| if e.MoveNext() then | ||
| remaining <- remaining - 1 | ||
| else | ||
| enoughElements <- false | ||
| if enoughElements then | ||
| while e.MoveNext() do | ||
| yield e.Current | ||
| } |
Conflicts: ReleaseNotes.md SafetyFirst.Specs/SeqSpec.fs
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
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.
No description provided.