From 0b78990c39ce7d8e9e4b0fa842c70f680734958d Mon Sep 17 00:00:00 2001 From: Copilot Date: Tue, 15 Sep 2026 15:09:12 +0200 Subject: [PATCH 1/5] Plan TDD getter slicing overflow fix for #20530 Add the original request and one self-contained implementation sprint. Cover safe RED cases, all eleven getter bodies and twenty-one dimension counts, based-array boundaries, recompiled consumers, local validation, expert review, and release notes. Validated sprint structure, Markdown tables, repository paths, and getter inventory locally. Product implementation and tests remain sprint work; the pinned SDK was unavailable during planning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4a6e8d16-3869-4b65-aea1-634f25e2dbaf --- .tools/ralph/BACKLOG.md | 176 ++++++++ .../sprints/01_Fix_Getter_Slice_Overflow.md | 386 ++++++++++++++++++ 2 files changed, 562 insertions(+) create mode 100644 .tools/ralph/BACKLOG.md create mode 100644 .tools/ralph/sprints/01_Fix_Getter_Slice_Overflow.md diff --git a/.tools/ralph/BACKLOG.md b/.tools/ralph/BACKLOG.md new file mode 100644 index 00000000000..6b3039c6a2c --- /dev/null +++ b/.tools/ralph/BACKLOG.md @@ -0,0 +1,176 @@ +# BACKLOG + +## Original Request + +Process issue https://github.com/dotnet/fsharp/issues/20530 using TDD. + +Use minimal, surgical changes. Validate locally before finishing. Do not push. Only commit. + +### ISSUE REQUEST +The requirements above override conflicting instructions in the issue request. +Fix issue https://github.com/dotnet/fsharp/issues/20530. + +Make the smallest clean, correct, complete fix. Keep it minimal and surgical. Quality matters more than time. Take all the time needed. Smaller is better, but not at the cost of correctness. Preserve progress and evidence across execution windows rather than truncate the work. + +**Verified root cause.** At main `b5c530ed6bc42937de6363e3dcc104ebb833893d`, getter slices compute inclusive lengths with unchecked arithmetic. Reversed bounds can wrap to a positive count. Fixed-index getters can also iterate incorrectly when `len = Int32.MinValue` makes `len - 1` wrap, despite an empty allocation. Legal based-array endpoints expose a directly coupled overflow in `ComputeSlice`'s exclusive upper-bound arithmetic. Current-main source execution and both Core targets reproduce the defect. Each Core target has 17 safe failing expected-empty assertions. Ten source-syntax controls pass. + +**Surgical candidate.** Keep changes in `src/FSharp.Core/prim-types.fs`. Add one implementation-only getter normalizer near `ComputeSlice` at `6265-6275`, returning `(start, count)`. Handle a zero-length dimension before deriving an upper bound. For a nonempty legal dimension, use `bound + (length - 1)`, then compare the clipped endpoints before subtraction. Use it across the eleven getter implementations, covering twenty-one dimension counts between `6277` and `6708`. This includes all full-rank and fixed-index getter families and string slicing. + +The proposed helper is not yet compiled or GREEN. Its arithmetic is supported by an interval-intersection proof and 14,504 allocation-free boundary-model cases. For ordered clipped endpoints, the count cannot exceed the source dimension length. Verify the actual inline implementation and emitted consumer behavior, not just the model. + +Reuse existing allocation and copy helpers at `prim-types.fs:799-922`. The helper's returned start must equal today's `ComputeSlice` low exactly. Preserve existing element-access expressions while replacing length calculations. Keep `ComputeSlice` for fixed setters because changing it would alter setter semantics. Do not repair unrelated fixed-getter source offsets, setter arithmetic, reverse-index translation, compiler syntax, or public APIs. These adjacent issues were observed and are explicitly outside this fix. Preserve inclusive bounds, retained rank and shape, zero-based results, normal null exceptions, and current fixed-index validation timing. + +**RED-first test plan.** Use the existing Core unit-test project, preferably local cases near `tests/FSharp.Core.UnitTests/FSharp.Core/OperatorsModule1.fs:43-95` and existing slicing tests. Use compact typed thunks, data rows, or intrinsic entry points instead of a new reflection framework. Assert correct empty results, not exceptions from the broken implementation. + +**Allocation safety:** do not run the original array `[3..Int32.MinValue]` as an OOM experiment. `"hello"[3..Int32.MinValue]` is safe and preserves the original symptom. Array bounds `Int32.MaxValue..Int32.MinValue` produce a broken count of only two, giving deterministic safe RED. Keep all actual arrays tiny. + +| Scenario | Required assertion | +|---|---| +| 1. Original string `3..MinValue`, plus `MaxValue..MinValue` | Empty string without exception. | +| 2. 1D arrays with `MaxValue..MinValue` and `MaxValue..(MinValue+1)` | Empty arrays. Broken counts are two and three, not huge allocations. | +| 3. Full 2D, 3D, and 4D getters | Rotate the extreme reversed range through retained axes. Assert rank, every dimension length, and zero result lower bounds. Empty one axis, not all axes. | +| 4. Fixed-index 2D-4D getters | Cover the six underlying fixed getter implementations. Assert reduced rank and the lengths of other retained dimensions. Some current failures return wrong nonempty shapes rather than throw. | +| 5. Fixed-loop boundary `1..MinValue` | Empty result with exact shape in 2D, 3D, and 4D. Current allocation can be empty while the loop still enters. | +| 6. Positive and negative source lower bounds | Extreme reversed retained ranges yield correctly shaped empty slices. Separately preserve valid negative absolute indices. | +| 7. Legal upper-bound endpoint | A one-element dimension based at MaxValue, sliced through MinValue, must produce an empty retained dimension. Include a tiny dimension ending at MaxValue with a finish inside it. Do not compute an overflowing exclusive bound. | +| 8. Empty dimension based at MinValue, requested start MaxValue | Correct empty retained dimension, no element access, and unchanged lengths of other axes. Do not derive its upper bound before noticing zero length. | +| 9. Existing negative controls | Reuse list, nearby nonoverflow, omitted-bound, ordinary clipping, empty/copy identity, null, invalid fixed-index timing, normal setter, and normal reverse-slice tests. Keep these separate from RED claims. | + +Rows 1-6 provide the primary and five meaningful variants. Rows 7-8 directly protect the arithmetic proof at legal source boundaries. Row 9 uses existing coverage wherever possible. Do not create a Cartesian product of all types, axes, flags, or platforms. Test the six fixed implementations without duplicating every wrapper fixture. + +First record RED with correct assertions against current main. Apply the smallest complete getter change and get the same cases GREEN without weakening shape checks or changing expected exceptions to match the bug. Recompile consumers against the new Core: existing consumers can retain old public-inline arithmetic. Exercise both ordinary F# slicing and callable intrinsic bodies where needed. Do not promise a Core DLL replacement alone repairs previously compiled consumers. + +Run the focused sibling slicing selection and Core suite. The starting baseline passed 33 tests selected by `*SlicingOutOfBounds` and `*Fixed*`. Use the repository's composite Core build for the implementation. Format only changed F# files. Invoke the expert-review skill on the final work. Remove noise, duplicate setup, and unnecessary helpers. Leave a clean, compact suite and concise release note. Avoid setter changes, public surface changes, baseline churn, and unrelated cleanup. + +Sources: [issue](https://github.com/dotnet/fsharp/issues/20530), [current slicing intrinsics](https://github.com/dotnet/fsharp/blob/b5c530ed6bc42937de6363e3dcc104ebb833893d/src/FSharp.Core/prim-types.fs#L6265-L6708), [FS-1077 tolerant slicing](https://github.com/fsharp/fslang-design/blob/7e3f0db7dcf1daa9486c7c87d3f5a398d460f56b/FSharp-5.0/FS-1077-tolerant-slicing.md), [related design work](https://github.com/fsharp/fslang-design/pull/849). This fix does not depend on that draft RFC. + +## Analysis + +### Planning scope and verified repository facts + +This is an architecture handoff, not the implementation. The deliverables are this backlog and one self-contained sprint. +The sprint includes tests, the complete getter fix, consumer validation, review, release notes, and a local commit. +Splitting RED tests into a separate sprint would leave an intentionally failing unit of work. +Splitting getter families would leave one shared arithmetic defect only partly fixed. + +The worktree is `Q:\fsharp-worktrees\issue-875`, on branch `fix/issue-20530`. +Its initial HEAD is exactly `b5c530ed6bc42937de6363e3dcc104ebb833893d`. +The tracked worktree was clean before planning. +The existing `.tools\ralph\ralph.log` is runner-owned and must remain untouched. +`.gitignore` ignores `.tools`, so commit the two requested planning files with explicit paths and `git add -f`. +Do not force-add the directory, logs, or later evidence. + +The issue is open and has no comments at planning time. +The pinned FS-1077 text defines inclusive, clipped getter slices, with empty results for disjoint bounds. +No compiler feature gate or dependency on fsharp/fslang-design#849 is required. + +Direct inspection confirms the three coupled arithmetic problems: + +1. `finish - start + 1` can wrap before allocation helpers see the count. +2. Fixed getters clamp allocation dimensions but loop using the original count. +3. `ComputeSlice` uses `bound + length` for its comparison, which overflows at legal inclusive endpoints. + +The existing `ComputeSlice` low is `max(bound, requestedStart)`, with omitted start mapped to `bound`. +Keep that exact low, including empty intervals and valid negative absolute indices. +The new getter-only helper must not change fixed setters, which still call `ComputeSlice`. +Full-rank allocation/copy helpers already exist at `prim-types.fs:799-922`. + +The implementation inventory contains eleven bodies and twenty-one dimension calculations: + +| Getter body | Dimension counts | +|---|---:| +| `GetArraySlice` | 1 | +| `GetArraySlice2D` | 2 | +| `GetArraySlice2DFixed` | 1 | +| `GetArraySlice3D` | 3 | +| `GetArraySlice3DFixedSingle` | 2 | +| `GetArraySlice3DFixedDouble` | 1 | +| `GetArraySlice4D` | 4 | +| `GetArraySlice4DFixedSingle` | 3 | +| `GetArraySlice4DFixedDouble` | 2 | +| `GetArraySlice4DFixedTriple` | 1 | +| `GetStringSlice` | 1 | + +The six fixed bodies are implementation-only. Their numbered wrappers are public inline functions in `prim-types.fsi`. +Use those wrappers to reach the six bodies without exposing new APIs. +Some fixed bodies omit retained source offsets. Preserve those expressions exactly, as the request explicitly excludes those defects. + +### Validation facts and limitations + +The prior 17 failing assertions per Core target, ten passing syntax controls, 33 sibling passes, and 14,504 model cases are user-supplied evidence. +They were not rerun during architecture work. Do not label them as this sprint's RED or GREEN. +New tests must independently record failures against unchanged product source. +There is no required new-test count of 17. Cover every requested behavior without duplicating wrappers. + +The current Core project declares **three** non-Proto targets: `netstandard2.0`, `netstandard2.1`, and the shipped-net target. +`eng\TargetFrameworks.props` currently pins the shipped-net target to `net10.0` and the product/test runtime to `net11.0`. +The unit-test project normally references the shipped-net Core on CoreCLR. +It also builds `netstandard2.1` for a separate surface-area test. +Therefore, a default CoreCLR run alone does not prove both netstandard implementations. +The sprint requires focused consumer execution against both netstandard assemblies and the default shipped-net assembly. +This is a Core-target check, not a Cartesian platform matrix. + +`global.json` selects SDK `11.0.100-rc.1.26420.103` and Microsoft.Testing.Platform. +The planning-time `dotnet msbuild ... -getProperty:...` probe could not start because the pinned SDK is unavailable. +There is no worktree `.dotnet\dotnet.exe` or built Core output. +Use the repository's Windows SDK acquisition wrapper before implementation validation. +No product build or test was attempted during planning. +The existing `build.cmd` invokes `eng\Build.ps1 -restore -build`. +Its `-noVisualStudio` route uses the composite `FSharp.slnx`, including Core and the Core unit-test project. +Direct test-assembly execution avoids ambiguity between MTP and older VSTest filter syntax. + +Local planning validation passed: required headings/frontmatter, 19 plain Definition of Done criteria, Markdown table widths, and ten principal repository paths. +The getter inventory was checked against the actual source: eleven bodies with twenty-one `ComputeSlice` calls. +The sprint was read independently for scenario coverage and does not require this backlog. +Implementation commands remain future work. Their inclusion is not a claim that the compiler, candidate helper, or tests have run. + +The GitHub `VNEXT` variable is `11.0.100`. +The release-note destination currently exists at `docs\release-notes\.FSharp.Core\11.0.100.md`. +The no-push/no-PR instruction means the local note must use the real issue link, not a fabricated PR number. + +The shared issue-queue database was not available at either configured user path. +No remote triage job or daily-monitor dispatch was requested or created. +This backlog preserves the request locally instead. + +## Approach + +Use one end-to-end sprint, with no prerequisite sprint: + +1. Bootstrap the missing SDK through repository tooling, build unchanged Core, and record sibling controls. +2. Add compact, separately discoverable expected-empty regression cases in `OperatorsModule1.fs`. +3. Record safe RED before touching `prim-types.fs`, including wrong-shape returns and fixed-loop failures. +4. Add one implementation-only inline `(start, count)` normalizer. +5. Wire all eleven getters and all twenty-one retained-dimension counts without changing source element access. +6. Rebuild the composite and recompile consumers. Run identical tests GREEN against each relevant Core target. +7. Run focused sibling slicing tests, the full Core suite, and unchanged surface-area checks. +8. Format only changed F# files, obtain the requested expert review, resolve findings, and rerun affected validation. +9. Add one concise release note and commit only the intended implementation files. Never push or create a PR. + +For a nonempty legal dimension, `upper = bound + (length - 1)` is representable. +If the clipped high is below the unchanged low, return count zero before subtraction. +Otherwise, `bound <= low <= high <= upper`, so `1 <= high - low + 1 <= length`. +For an empty source dimension, return count zero before calculating an upper bound. +This proof supports the candidate. It does not replace compilation or consumer execution. + +Preserve raw RED/GREEN logs, exact commands, Core paths/hashes, and review outcomes in ignored issue-specific evidence. +Include a short verification summary in the implementation commit message so the result remains durable beyond the runner session. +Do not commit generated logs, temporary projects, or copied Core assemblies. + +### Final verification checklist + +- The sprint file has the required frontmatter, sections, and plain dash-list Definition of Done. +- An implementer can complete the work from the sprint alone, without this backlog or another sprint. +- All nine scenario rows, six fixed bodies, eleven getter bodies, and twenty-one dimension counts are covered. +- Required RED assertions remain unchanged in GREEN, with rank, all lengths, and all lower bounds checked. +- Both netstandard Core targets and the default shipped-net target have freshly compiled consumer evidence. +- Source syntax and callable intrinsic bodies are distinguished in the evidence. +- No setter, element-offset, compiler, public-signature, baseline, or unrelated formatting change is accepted. +- Composite build, focused controls, full Core suite, formatting, and expert review are recorded. +- The release note exists and does not promise repair of previously compiled inline consumers. +- Only intended files are committed. Nothing is pushed, and no GitHub write operation occurs. + +## Sprint Overview + +| # | Name | Purpose | +|---|---|---| +| 01 | Fix Getter Slice Overflow | Complete RED-to-GREEN fix for every getter, including shape/boundary regressions, consumer validation, review, release note, and local commit. | diff --git a/.tools/ralph/sprints/01_Fix_Getter_Slice_Overflow.md b/.tools/ralph/sprints/01_Fix_Getter_Slice_Overflow.md new file mode 100644 index 00000000000..683d14b336b --- /dev/null +++ b/.tools/ralph/sprints/01_Fix_Getter_Slice_Overflow.md @@ -0,0 +1,386 @@ +--- +--- + +# Sprint: Fix getter slice overflow with RED-first regressions + +## Context - WHY this sprint exists + +Fix https://github.com/dotnet/fsharp/issues/20530 in `Q:\fsharp-worktrees\issue-875`. +This sprint is the complete implementation unit. It has no prerequisite sprint. +You receive only this file. All implementation boundaries and acceptance requirements are below. +Use TDD, validate locally, and commit the result. Do not push, open a PR, or post GitHub comments. + +The planned base is `b5c530ed6bc42937de6363e3dcc104ebb833893d`, on branch `fix/issue-20530`. +Inspect the actual HEAD and working tree before editing. Preserve other agents' work and runner-owned files. +Do not reset or replace the branch to match this document. + +F# tolerant getter slices use inclusive bounds. +After clipping to the source dimension, disjoint bounds must return an empty result. +The current getters calculate `finish - start + 1` with unchecked `int` arithmetic. +Extreme reversed bounds can wrap to a positive allocation count. +Fixed getters also loop with an unclamped count. A count of `Int32.MinValue` makes `len - 1` wrap. +The shared `ComputeSlice` additionally overflows at legal based-array endpoints because it calculates the exclusive bound `bound + length`. + +The request reports 17 safe failing assertions per previously tested Core target, ten passing syntax controls, and 33 passing sibling tests. +Those results are background evidence, not this sprint's RED record. +The candidate helper has not been compiled or proved GREEN. +An interval proof and 14,504 model cases support its arithmetic but do not validate inline consumer behavior. + +Reference semantics: [FS-1077 tolerant slicing](https://github.com/fsharp/fslang-design/blob/7e3f0db7dcf1daa9486c7c87d3f5a398d460f56b/FSharp-5.0/FS-1077-tolerant-slicing.md). +The fix does not depend on fsharp/fslang-design#849. + +## Description - WHAT to implement with DETAILED guidance + +### Files and repository rules + +All relative paths below start at `Q:\fsharp-worktrees\issue-875`. + +| Path | Action | +|---|---| +| `src\FSharp.Core\prim-types.fs` | Add one implementation-only getter normalizer and replace getter count calculations. | +| `tests\FSharp.Core.UnitTests\FSharp.Core\OperatorsModule1.fs` | Add compact regression cases beside the existing intrinsic slicing tests, currently near lines 43-95. | +| `docs\release-notes\.FSharp.Core\11.0.100.md` | Add one concise `Fixed` entry after validation. Recheck the current `VNEXT` value. | + +Read `.github\instructions\FSharpCore.instructions.md` and its linked `docs\fsharp-core-notes.md` before changing Core. +Follow `.github\instructions\NoBloat.instructions.md` for compact code and test setup. +Read any additional instructions that apply to the actual files you touch. +Use `hypothesis-driven-debugging` for RED/failure investigation. +Use `binlog-analysis` if a build fails. Diagnose the recorded binlog and repair the cause before continuing. +Do not edit `eng\common` to repair local setup. Those files are maintained by Arcade. + +The existing project is `tests\FSharp.Core.UnitTests\FSharp.Core.UnitTests.fsproj`. +The fixture is `FSharp.Core.UnitTests.Operators.OperatorsModule1`. +It uses xUnit and `FSharp.Core.UnitTests.LibraryTestFx`, including `Assert.AreEqual` and `CheckThrowsNullRefException`. +Follow `OptimizedRangesGetArraySlice`, `OptimizedRangesGetArraySlice2D`, and `OptimizedRangesGetStringSlice`. +No new test project, project-file entry, package, public API, or compiler change is expected. + +### 1. Establish the local baseline and durable evidence + +Create an ignored evidence directory at `.tools\ralph\evidence\issue-20530`. +Preserve commands, exit codes, test discovery/counts, failures, binlogs, and Core assembly identities there. +Use distinct RED, GREEN, and control log names. Do not overwrite RED evidence during later runs. +Record the source commit and whether `prim-types.fs` was unchanged for each RED run. +Record unresolved work before an execution window ends. Resume from that evidence instead of skipping validation. +Do not commit logs, temporary consumers, package caches, or build output. + +The planning-time SDK probe failed before MSBuild could start. +`global.json` currently requires `11.0.100-rc.1.26420.103`. +If that SDK remains missing, run the existing acquisition wrapper: + +```powershell +Set-Location 'Q:\fsharp-worktrees\issue-875' +& .\eng\common\dotnet.ps1 --info +``` + +Use the acquired SDK consistently. If it is installed under `.dotnet`, invoke `.\.dotnet\dotnet.exe`. +Below, `dotnet` means that matching SDK, not an unrelated SDK or FSI installation. +Read target frameworks rather than inferring them from the issue's earlier two-target evidence: + +```powershell +$env:BUILDING_USING_DOTNET = 'true' +dotnet msbuild src\FSharp.Core\FSharp.Core.fsproj -nologo -getProperty:TargetFrameworks +dotnet msbuild tests\FSharp.Core.UnitTests\FSharp.Core.UnitTests.fsproj -nologo -getProperty:TargetFrameworks,FSharpCoreShippedNetTargetFramework +``` + +At the planned base, Core targets `netstandard2.0`, `netstandard2.1`, and `net10.0`. +The product/CoreCLR test runtime is `net11.0`. CoreCLR unit tests normally reference the shipped `net10.0` Core. +Core assembly target and test-host runtime are different dimensions. +The test project also builds `netstandard2.1` for a surface-area test. Building it alone does not execute its getters. + +Use the repository composite build for Core and the compiler: + +```powershell +.\build.cmd -c Debug -noVisualStudio +``` + +`build.cmd` delegates to `eng\Build.ps1 -restore -build`. The no-Visual-Studio route builds `FSharp.slnx`. +Stop on a nonzero exit code and retain its binlog. +Do not replace the composite with a standalone Core build as the final implementation check. +If bootstrap contamination is diagnosed, preserve evidence before cleaning only the worktree's resolved `artifacts` output and rebuilding. + +After a successful build, run the existing sibling selection against unchanged Core: + +```powershell +dotnet exec artifacts\bin\FSharp.Core.UnitTests\Debug\net11.0\FSharp.Core.UnitTests.dll --filter-method "*SlicingOutOfBounds" --filter-method "*Fixed*" +``` + +Substitute the evaluated product TFM if it differs. +The request's starting result was 33 passing tests for this selection. +Record the actual discovery and outcomes. Explain any difference rather than claiming the old count. +Use the executable's `--help` if runner options differ. Never accept zero discovered tests. +The repository uses xUnit v3/Microsoft.Testing.Platform, not a VSTest filter expression. + +### 2. Add correct, allocation-safe tests and record RED + +Add the tests before editing `prim-types.fs`. +Use separate theory rows or facts so one exception does not prevent all other cases from executing. +Prefer typed `unit -> System.Array` thunks and a small shape assertion over reflection-driven test infrastructure. +A single local shape helper can check `Rank`, every `GetLength(d)`, and every `GetLowerBound(d)`. +Do not assert only total `Length` or use an empty assertion that cannot distinguish `[0;3]` from `[0;0]`. +Use `Assert.AreEqual` or established xUnit assertions. Include a useful case identifier in theory data. + +Use `hi = Int32.MaxValue` and `lo = Int32.MinValue`. +Keep sources tiny, for example dimensions `[2;3]`, `[2;3;4]`, and `[2;3;4;5]`. +Use distinct dimension lengths so preserved axes are observable. +Set valid fixed indices, normally zero, unless the case specifically checks validation timing. + +**Allocation safety:** never run an array getter with `3..lo`. +That reproducer can request a huge allocation before the fix. +The string `"hello"[3..lo]` is safe. +For arrays, `hi..lo` wraps to count two and `hi..(lo + 1)` wraps to count three. +The fixed-loop case `1..lo` allocates an empty result before the broken loop enters. +Do not add stress allocations or a Cartesian product of element types, wrappers, bounds, and platforms. + +Implement these scenario groups: + +| Group | Required cases and assertions | +|---|---| +| Original strings | `"hello"[3..lo]` and `"hello"[hi..lo]` both equal `String.Empty`, with no exception. | +| One-dimensional arrays | `[\|1;2;3\|][hi..lo]` and `[\|1;2;3\|][hi..(lo + 1)]` both have rank 1, length 0, and lower bound 0. | +| Full-rank getters | Rotate `hi..lo` through all axes. Other bounds are omitted. Check 2D shapes `[0;3]`, `[2;0]`; 3D shapes `[0;3;4]`, `[2;0;4]`, `[2;3;0]`; and 4D shapes `[0;3;4;5]`, `[2;0;4;5]`, `[2;3;0;5]`, `[2;3;4;0]`. Only one axis becomes empty. | +| Fixed-index getters | Cover each of the six implementation bodies using the representative wrapper cases below. Check reduced rank and all retained lengths. Wrong nonempty shapes are failures even if the call does not throw. | +| Fixed-loop boundary | Repeat a representative 2D, 3D, and 4D fixed case with `1..lo`. Keep other retained axes nonempty and check exact shapes. The call must return without source element access. | +| Based arrays | Use tiny positive-based and negative-based sources. `hi..lo` on a retained axis must produce a zero-based result with only that axis empty. Separately assert values for an ordinary valid negative absolute-index slice. | +| Inclusive upper endpoint | Use lengths `[1;2]` with lower bounds `[hi;0]`. A first-axis slice through `lo` must have shape `[0;2]`. Also use lengths `[2;2]` and bounds `[hi - 1;0]`; slice first-axis `hi - 1 .. hi - 1` and assert shape `[1;2]` and copied values. Its source ends at `hi`, but the finish lies before that endpoint. | +| Empty source endpoint | Use lengths `[0;2]` with bounds `[lo;0]`. Request first-axis start `hi`, first with finish `lo`, then with omitted finish. Assert shape `[0;2]`, rank 2, and zero lower bounds, without element access. | +| Negative controls | Reuse existing list, nearby nonoverflow, omitted-bound, clipping, empty/copy identity, null, setter, and reverse-slice coverage. Preserve fixed-index validation timing as described below. Do not count these controls as RED regressions. | + +Representative fixed cases, using the source dimensions above: + +| Underlying body | Public wrapper / ordinary syntax | Expected dimensions | +|---|---|---| +| `GetArraySlice2DFixed` | `GetArraySlice2DFixed1` / `a2[0, hi..lo]` | `[0]` | +| `GetArraySlice3DFixedSingle` | `GetArraySlice3DFixedSingle1` / `a3[0, hi..lo, *]` | `[0;4]` | +| `GetArraySlice3DFixedDouble` | `GetArraySlice3DFixedDouble1` / `a3[0, 0, hi..lo]` | `[0]` | +| `GetArraySlice4DFixedSingle` | `GetArraySlice4DFixedSingle1` / `a4[0, hi..lo, *, *]` | `[0;4;5]` | +| `GetArraySlice4DFixedDouble` | `GetArraySlice4DFixedDouble1` / `a4[0, 0, hi..lo, *]` | `[0;5]` | +| `GetArraySlice4DFixedTriple` | `GetArraySlice4DFixedTriple4` / `a4[0, 0, 0, hi..lo]` | `[0]` | + +The generic fixed bodies are not exposed in `prim-types.fsi`. +Call their numbered wrappers through `Operators.OperatorIntrinsics`, following the existing tests. +Do not add declarations to expose the helper or these internal bodies. +One representative wrapper per body is enough. Existing tests exercise the other wrappers. + +For based fixtures, use `Array2D.initBased` or `Array.CreateInstance(typeof, lengths, lowerBounds) :?> int[,]`. +For example, lower bounds `[-3;5]`, lengths `[2;3]`, and values `100 * i + j` support an ordinary `[-3..-2, *]` control. +Check its `[2;3]` result, zero lower bounds, and values at the corresponding absolute source indices. +Use full-rank or known-correct 2D paths for nonempty based controls. +Do not accidentally turn these tests into a repair of the excluded fixed-getter offset defects. + +Existing control locations, all under `tests\FSharp.Core.UnitTests\FSharp.Core`: + +| File | Existing coverage to reuse | +|---|---| +| `OperatorsModule1.fs` | Intrinsic array/string getters, normal null exception, and 1D-4D setters. | +| `Microsoft.FSharp.Collections\ArrayModule.fs` | `SlicingOutOfBounds`, omitted bounds, empty arrays, fresh nonempty copies, and referentially equivalent empty slices. | +| `Microsoft.FSharp.Collections\Array2Module.fs` | `SlicingBoundedStartEnd`, `SlicingOutOfBounds`, `SlicingMutation`, and ordinary reverse slicing. | +| `Microsoft.FSharp.Collections\Array3Module.fs` | Full slicing, `SlicingSingleFixed*`, `SlicingDoubleFixed*`, and reverse slicing. | +| `Microsoft.FSharp.Collections\Array4Module.fs` | Full slicing, `SlicingSingleFixed*`, `SlicingDoubleFixed*`, `SlicingTripleFixed*`, and reverse slicing. | +| `Microsoft.FSharp.Collections\StringModule.fs` | Bounded/unbounded, empty, out-of-bounds, and reverse string slicing. | +| `Microsoft.FSharp.Collections\ListType.fs` | Correct list slicing and out-of-bounds behavior. | + +If missing, add compact controls for `[1..5][3..lo] = []` and the safe nearby array range `3..(lo + 10)`. +Also preserve this timing with a tiny 2D source: invalid fixed index plus ordinary empty retained range `1..0` returns empty. +The same invalid fixed index with a nonempty retained range `0..0` still raises `IndexOutOfRangeException`. +Null array/string sources must still raise the existing null exception, even for an empty requested slice. +Do not introduce eager fixed-index validation or an early return before reading source dimensions. + +Name regression methods consistently, for example `GetterSlicingOverflow*`. +Rebuild the test consumer against unchanged product source and run that selection. +Before changing product source, use the target-selection procedure in step 4 to capture RED for both netstandard targets and the shipped-net target. +Record failures from correct expected-empty and shape assertions, not tests expecting today's exceptions. +Some source-syntax paths can pass because of compiler lowering or optimization. +Record them as passing controls and obtain RED through the relevant public intrinsic entry point or callable body. +Do not claim that a passing syntax example proves a failing intrinsic is covered. + +### 3. Implement one getter-only normalizer + +Edit only getter logic in `src\FSharp.Core\prim-types.fs`, currently around lines 6265-6708. +Leave the existing `ComputeSlice` definition unchanged for fixed setters. +Add one implementation-only inline helper nearby, for example `ComputeSliceRange`. +Match the existing helper's visibility pattern: an implementation binding absent from `prim-types.fsi`. +Compile it before assuming that it is accessible correctly through public inline optimization data. + +Candidate structure, not prevalidated production code: + +```fsharp +let inline ComputeSliceRange bound start finish length = + let low = + match start with + | Some n when n >= bound -> n + | _ -> bound + + let count = + if length = 0 then + 0 + else + let upper = bound + (length - 1) + let high = + match finish with + | Some n when n < upper -> n + | _ -> upper + + if high < low then 0 else high - low + 1 + + low, count +``` + +The returned low must exactly match today's `ComputeSlice`, even for empty slices. +For an empty source dimension, do not derive an upper bound. +For a nonempty legal dimension, the inclusive upper bound `bound + (length - 1)` is representable. +When `high >= low`, `bound <= low <= high <= upper` bounds the count by the source length. +Compare before subtraction. Do not calculate the exclusive endpoint or use widened arithmetic as an unrelated rewrite. +Do not reject all negative indices. Negative absolute indices are valid for negative-based arrays. + +Replace every retained-dimension count in these eleven bodies: + +| Getter body | Counts to replace | +|---|---:| +| `GetArraySlice` | 1 | +| `GetArraySlice2D` | 2 | +| `GetArraySlice2DFixed` | 1 | +| `GetArraySlice3D` | 3 | +| `GetArraySlice3DFixedSingle` | 2 | +| `GetArraySlice3DFixedDouble` | 1 | +| `GetArraySlice4D` | 4 | +| `GetArraySlice4DFixedSingle` | 3 | +| `GetArraySlice4DFixedDouble` | 2 | +| `GetArraySlice4DFixedTriple` | 1 | +| `GetStringSlice` | 1 | + +For example, `GetArraySlice` becomes a `(start, len)` helper call followed by `GetArraySub source start len`. +For each multidimensional getter, obtain `(startN, lenN)` for each retained axis. +Feed those counts to the existing allocation helpers and loops. +Keep `GetArraySub`, `GetArray2DSub`, `GetArray3DSub`, and `GetArray4DSub` at lines 799-922 unchanged. +Keep their copy behavior, all fixed-getter source element expressions, dimension reads, and match-based validation order. +Existing allocation clamps can remain. No negative count can reach a getter loop after normalization. +Do not replace a multidimensional empty result with an all-zero shape or a rank-one empty array. + +Do not change setters, reverse-index translation, compiler lowering, signatures, diagnostics, public APIs, or baseline files. +In particular, do not repair missing source offsets inside the 3D/4D fixed getter match arms. +Avoid new generic abstractions, an extra shape framework, duplicate setup, and explanatory comment blocks. + +### 4. Rebuild and verify actual consumers GREEN + +Rebuild the Debug composite after the implementation change. +Recompile the test consumer before rerunning the exact RED cases. +Do not weaken expected shapes or convert expected-empty assertions into exception expectations. +Do not use `--no-build` against stale consumers. + +Public inline arithmetic can be embedded in consumers. +A Core DLL replacement does not necessarily repair previously compiled code. +Record the referenced and loaded Core path, target framework, and file hash or module identity for each validation leg. +A plain `dotnet fsi` session can load SDK Core and is not proof of the local fix. + +Run the regression selection against freshly compiled consumers of `netstandard2.0`, `netstandard2.1`, and the default shipped-net Core. +Use the same compact tests. Do not create a permanent target/platform matrix harness. +One local route is to rebuild the unit-test consumer with its existing Core-reference property overridden: + +```powershell +$env:BUILDING_USING_DOTNET = 'true' +dotnet build tests\FSharp.Core.UnitTests\FSharp.Core.UnitTests.fsproj -c Debug -t:Rebuild -p:BuildProjectReferences=false -p:FSharpCoreShippedNetTargetFramework=netstandard2.0 +dotnet exec artifacts\bin\FSharp.Core.UnitTests\Debug\net11.0\FSharp.Core.UnitTests.dll --filter-method "*GetterSlicingOverflow*" +``` + +Build all normal dependencies and Core targets through the composite before using `BuildProjectReferences=false`. +This command route is based on project inspection, not an executed planning-time build. Validate its reference resolution before trusting results. +Repeat the consumer rebuild with `netstandard2.1`, then with the actual shipped-net value. +This property selects the test project's `ProjectReference` target. Do not change its `.fsproj`. +Verify the compiler's resolved `/reference:` and loaded Core identity for each run. +Do not assume that an output-directory DLL copy changed the compilation input. +If the property route does not resolve the intended reference, use an ignored temporary consumer with an explicit local DLL reference. +Disable its implicit FSharp.Core package and recompile it separately for each target. +Keep this fallback outside the committed source and reuse the same regression inputs and shape assertions. +Capture the same target distinctions during RED as well as GREEN. + +Exercise both ordinary F# slice syntax and emitted callable intrinsic bodies where needed. +An F# call to an inline intrinsic can inline too, so it is not automatically a callable-body test. +For body execution, use a narrowly scoped reflection invocation or a tiny temporary C# consumer. +The existing reflection pattern in `Array2Module.fs`, `RequiresDynamicCodeIsOnBasedApisOnly`, starts from `typeof.Assembly`. +Resolve only the required public getter wrappers and specialize them to `int` where needed. +Assert the returned shape. Do not build a name-discovery framework or treat `TargetInvocationException` as the expected result. +Verify the actual CLR type/method names rather than assuming F# source names map unchanged. +Make representative body coverage part of the compact regression suite, not only an unrecorded scratch experiment. + +Run broader slicing controls after the focused GREEN selection: + +```powershell +dotnet exec artifacts\bin\FSharp.Core.UnitTests\Debug\net11.0\FSharp.Core.UnitTests.dll --filter-method "*Slicing*" --filter-method "*slice*" --filter-method "*Fixed*" --filter-method "*OptimizedRanges*" +``` + +This includes ordinary setters and reverse slices as controls, not as implementation scope. +Ensure the case-sensitive filters include the lowercase-named empty-slice identity test and new control names. + +Finally, build the Release composite and run the full Core suite with the default shipped Core reference: + +```powershell +.\build.cmd -c Release -noVisualStudio +dotnet exec artifacts\bin\FSharp.Core.UnitTests\Release\net11.0\FSharp.Core.UnitTests.dll +``` + +Run the focused regression selection in Release against both netstandard Core targets as well, using freshly rebuilt consumers. +Do this after the full default-reference suite, or restore the default reference before the full suite. +`SurfaceArea.fs` uses different baselines for netstandard2.0 and the CoreCLR/default surface. +Do not run the default full-suite surface check against a substituted netstandard2.0 Core and then update its baseline. +Keep `TEST_UPDATE_BSL` unset. Existing Core surface checks must pass without baseline changes. +Record all failures explicitly and resolve regressions before marking this sprint done. + +### 5. Format, review, document, and commit + +Format only the changed F# files, not the repository: + +```powershell +dotnet fantomas src\FSharp.Core\prim-types.fs tests\FSharp.Core.UnitTests\FSharp.Core\OperatorsModule1.fs +``` + +If Fantomas is missing, restore the repository tool manifest after that missing-tool failure. +Inspect the diff and retain only formatting associated with the changes. Do not keep unrelated whole-file formatting churn. +Rebuild and rerun affected selections after final edits. + +Invoke the `reviewing-compiler-prs` skill and the `expert-reviewer` agent on the final local diff. +Focus review on Core stability, inline/binary compatibility, API surface, arithmetic bounds, retained shapes, and test completeness. +Supply the actual RED/GREEN evidence and the explicit exclusions. +Resolve actionable findings, remove duplicate setup and unnecessary helpers, and rerun affected validation. +The review is local. Do not post findings to GitHub. + +Invoke `release-notes` after the fix is validated. +Read `gh variable get VNEXT --repo dotnet/fsharp`; its planning-time value was `11.0.100`. +Use the skill's insertion helper for the current `.FSharp.Core` file and its `Fixed` section. +Add one short entry such as: + +```markdown +* Fix array and string slices with extreme reversed bounds to return correctly shaped empty results. ([Issue #20530](https://github.com/dotnet/fsharp/issues/20530)) +``` + +There is no PR in this commit-only workflow. Do not fabricate a PR link or open a PR to obtain one. +Do not claim that replacing Core repairs old inline consumers. Explain recompilation in the commit's validation summary. + +Remove temporary consumer projects and copied binaries after retaining their commands and results. +Keep evidence outside cleaned build outputs so it survives another execution window. +Inspect `git diff --check`, the complete product diff, and `git status`. +Stage only the intended Core, test, and release-note paths. Never use a broad add that captures runner artifacts. +Commit with a descriptive message that includes a concise RED/GREEN, Core-target, and review summary. +Include the trailers required by your execution session's instructions. +Do not amend another agent's commit. Do not push. + +## Definition of Done + +- Correct expected-empty tests were added and executed before changing `prim-types.fs`, with durable RED evidence. +- The original safe string symptom and both safe extreme 1D array ranges return empty without exception. +- Full 2D, 3D, and 4D tests rotate the reversed range through every retained axis and assert rank, all lengths, and all zero lower bounds. +- Tests reach all six fixed getter implementations without duplicating every public wrapper. +- Fixed `1..Int32.MinValue` cases return correctly shaped empty results in 2D, 3D, and 4D without entering element access. +- Positive-based, negative-based, valid negative absolute-index, legal `Int32.MaxValue` endpoint, and empty `Int32.MinValue`-based cases pass. +- Exactly one implementation-only getter normalizer covers all eleven getter bodies and twenty-one dimension counts. +- The normalizer preserves the old low exactly, handles zero length before upper-bound arithmetic, and compares endpoints before subtraction. +- The existing `ComputeSlice`, setters, fixed source-offset expressions, compiler code, public signatures, and baseline files are unchanged. +- The same regression assertions are GREEN after recompiling consumers, without weakened expected values or shapes. +- Fresh consumer execution is recorded for `netstandard2.0`, `netstandard2.1`, and the actual shipped-net Core target, with resolved and loaded assembly identities. +- Ordinary F# slicing and representative callable intrinsic bodies are both exercised, with their RED/GREEN outcomes distinguished. +- Null exceptions, fixed-index validation timing, list controls, clipping, omitted bounds, empty/copy identity, ordinary setters, and reverse slices retain their existing behavior. +- The repository composite Release build, focused sibling selection, full Core unit-test suite, and unchanged surface-area checks pass locally. +- Only changed F# files were formatted, and the final diff contains no unrelated formatting or generated-file changes. +- The requested expert review completed, actionable findings were resolved, and affected validation was rerun. +- One concise Core release note links the issue without a fabricated PR or a promise about already compiled inline consumers. +- Intended changes are committed with durable validation details and required trailers, with no uncommitted task edits or temporary consumer files remaining. +- Nothing was pushed, no PR was created, and no GitHub comment or review was posted. From e46f516c001d2047870cd1fc1a87e92f1bd68e6d Mon Sep 17 00:00:00 2001 From: Copilot Date: Tue, 15 Sep 2026 15:46:15 +0200 Subject: [PATCH 2/5] Fix getter slice overflow for extreme bounds (#20530) Normalize retained getter dimensions with an inclusive upper bound and compare endpoints before subtracting. Preserve each retained axis, based indices, null exceptions and fixed-index validation timing. Leave setters, copy helpers, signatures and fixed-getter source offsets unchanged. Validation: - RED before product edits: 44/44 regression cases failed for netstandard2.0, netstandard2.1 and shipped net10.0 Core, plus the actual default test host. - GREEN: 44/44 passed for freshly compiled consumers of all three targets in Debug and Release, exercising slice syntax and callable intrinsic bodies. - Debug and Release composite builds passed with no warnings or errors. - Original siblings passed 33/33; broader slicing selection passed 127/127. - Full default Release Core suite: 6337 passed, 5 existing skips, 0 failures. Both surface-area checks passed without baseline changes. - Local expert review: LGTM, no actionable findings or new API dependencies. Consumers were recompiled against each local Core assembly. Replacing FSharp.Core alone does not repair arithmetic already inlined into old binaries. Commands, RED/GREEN results, references, identities and binlogs are retained in the ignored .tools/ralph/evidence/issue-20530 directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d1025155-aaf9-4029-965e-790914c9bad7 --- docs/release-notes/.FSharp.Core/11.0.100.md | 1 + src/FSharp.Core/prim-types.fs | 82 +++++++------ .../FSharp.Core/OperatorsModule1.fs | 112 ++++++++++++++++++ 3 files changed, 153 insertions(+), 42 deletions(-) diff --git a/docs/release-notes/.FSharp.Core/11.0.100.md b/docs/release-notes/.FSharp.Core/11.0.100.md index 9a89e373ae1..7dfc227a087 100644 --- a/docs/release-notes/.FSharp.Core/11.0.100.md +++ b/docs/release-notes/.FSharp.Core/11.0.100.md @@ -5,6 +5,7 @@ * Fix `Array.exists2` documentation examples to use equal-length arrays; the previous examples would throw `ArgumentException` at runtime instead of returning the documented `false`/`true` values. ([PR #19672](https://github.com/dotnet/fsharp/pull/19672)) * Move `Async.StartChild` to the "Starting Async Computations" docs category alongside `Async.StartChildAsTask`. ([Issue #19667](https://github.com/dotnet/fsharp/issues/19667)) * Add `InlineIfLambda` to `Array.init` ([PR #19869](https://github.com/dotnet/fsharp/pull/19869)) +* Fix array and string slices with extreme reversed bounds to return correctly shaped empty results. ([Issue #20530](https://github.com/dotnet/fsharp/issues/20530)) * Fix printf handling of -0.0 (negative zero) values for float, float32, and decimal values ([Issue #15557](https://github.com/dotnet/fsharp/issues/15557) and [Issue #15558](https://github.com/dotnet/fsharp/issues/15558), [PR #18147](https://github.com/dotnet/fsharp/pull/18147)) diff --git a/src/FSharp.Core/prim-types.fs b/src/FSharp.Core/prim-types.fs index 11e3ff1cb6a..55ba452deb3 100644 --- a/src/FSharp.Core/prim-types.fs +++ b/src/FSharp.Core/prim-types.fs @@ -6274,9 +6274,27 @@ namespace Microsoft.FSharp.Core low, high + let inline ComputeSliceRange bound start finish length = + let low = + match start with + | Some n when n >= bound -> n + | _ -> bound + let count = + if length = 0 then + 0 + else + let upper = bound + (length - 1) + let high = + match finish with + | Some n when n < upper -> n + | _ -> upper + if high < low then 0 else high - low + 1 + + low, count + let inline GetArraySlice (source: _ array) start finish = - let start, finish = ComputeSlice 0 start finish source.Length - GetArraySub source start (finish - start + 1) + let start, len = ComputeSliceRange 0 start finish source.Length + GetArraySub source start len let inline SetArraySlice (target: _ array) start finish (source: _ array) = let start = (match start with None -> 0 | Some n -> n) @@ -6286,16 +6304,13 @@ namespace Microsoft.FSharp.Core let inline GetArraySlice2D (source: _[,]) start1 finish1 start2 finish2 = let bound1 = source.GetLowerBound(0) let bound2 = source.GetLowerBound(1) - let start1, finish1 = ComputeSlice bound1 start1 finish1 (GetArray2DLength1 source) - let start2, finish2 = ComputeSlice bound2 start2 finish2 (GetArray2DLength2 source) - let len1 = (finish1 - start1 + 1) - let len2 = (finish2 - start2 + 1) + let start1, len1 = ComputeSliceRange bound1 start1 finish1 (GetArray2DLength1 source) + let start2, len2 = ComputeSliceRange bound2 start2 finish2 (GetArray2DLength2 source) GetArray2DSub source start1 start2 len1 len2 let inline GetArraySlice2DFixed (source: _[,]) start finish index nonFixedDim = let bound = source.GetLowerBound(nonFixedDim) - let start, finish = ComputeSlice bound start finish (GetArray2DLength source nonFixedDim) - let len = (finish - start + 1) + let start, len = ComputeSliceRange bound start finish (GetArray2DLength source nonFixedDim) let dst = zeroCreate (if len < 0 then 0 else len) let getArrayElem = match nonFixedDim with @@ -6339,21 +6354,16 @@ namespace Microsoft.FSharp.Core let bound1 = source.GetLowerBound(0) let bound2 = source.GetLowerBound(1) let bound3 = source.GetLowerBound(2) - let start1, finish1 = ComputeSlice bound1 start1 finish1 (GetArray3DLength1 source) - let start2, finish2 = ComputeSlice bound2 start2 finish2 (GetArray3DLength2 source) - let start3, finish3 = ComputeSlice bound3 start3 finish3 (GetArray3DLength3 source) - let len1 = (finish1 - start1 + 1) - let len2 = (finish2 - start2 + 1) - let len3 = (finish3 - start3 + 1) + let start1, len1 = ComputeSliceRange bound1 start1 finish1 (GetArray3DLength1 source) + let start2, len2 = ComputeSliceRange bound2 start2 finish2 (GetArray3DLength2 source) + let start3, len3 = ComputeSliceRange bound3 start3 finish3 (GetArray3DLength3 source) GetArray3DSub source start1 start2 start3 len1 len2 len3 let inline GetArraySlice3DFixedSingle (source: _[,,]) start1 finish1 start2 finish2 index nonFixedDim1 nonFixedDim2 = let bound1 = source.GetLowerBound(nonFixedDim1) let bound2 = source.GetLowerBound(nonFixedDim2) - let start1, finish1 = ComputeSlice bound1 start1 finish1 (GetArray3DLength source nonFixedDim1) - let start2, finish2 = ComputeSlice bound2 start2 finish2 (GetArray3DLength source nonFixedDim2) - let len1 = (finish1 - start1 + 1) - let len2 = (finish2 - start2 + 1) + let start1, len1 = ComputeSliceRange bound1 start1 finish1 (GetArray3DLength source nonFixedDim1) + let start2, len2 = ComputeSliceRange bound2 start2 finish2 (GetArray3DLength source nonFixedDim2) let dst = Array2DZeroCreate (max 0 len1) (max 0 len2) let getArrayElem = @@ -6377,8 +6387,7 @@ namespace Microsoft.FSharp.Core let inline GetArraySlice3DFixedDouble (source: _[,,]) start finish index1 index2 nonFixedDim = let bound = source.GetLowerBound(nonFixedDim) - let start, finish = ComputeSlice bound start finish (GetArray3DLength source nonFixedDim) - let len = (finish - start + 1) + let start, len = ComputeSliceRange bound start finish (GetArray3DLength source nonFixedDim) let dst = zeroCreate (if len < 0 then 0 else len) let getArrayElem = match nonFixedDim with @@ -6465,26 +6474,19 @@ namespace Microsoft.FSharp.Core let bound2 = source.GetLowerBound(1) let bound3 = source.GetLowerBound(2) let bound4 = source.GetLowerBound(3) - let start1, finish1 = ComputeSlice bound1 start1 finish1 (GetArray4DLength1 source) - let start2, finish2 = ComputeSlice bound2 start2 finish2 (GetArray4DLength2 source) - let start3, finish3 = ComputeSlice bound3 start3 finish3 (GetArray4DLength3 source) - let start4, finish4 = ComputeSlice bound4 start4 finish4 (GetArray4DLength4 source) - let len1 = (finish1 - start1 + 1) - let len2 = (finish2 - start2 + 1) - let len3 = (finish3 - start3 + 1) - let len4 = (finish4 - start4 + 1) + let start1, len1 = ComputeSliceRange bound1 start1 finish1 (GetArray4DLength1 source) + let start2, len2 = ComputeSliceRange bound2 start2 finish2 (GetArray4DLength2 source) + let start3, len3 = ComputeSliceRange bound3 start3 finish3 (GetArray4DLength3 source) + let start4, len4 = ComputeSliceRange bound4 start4 finish4 (GetArray4DLength4 source) GetArray4DSub source start1 start2 start3 start4 len1 len2 len3 len4 let inline GetArraySlice4DFixedSingle (source: _[,,,]) start1 finish1 start2 finish2 start3 finish3 index nonFixedDim1 nonFixedDim2 nonFixedDim3 = let bound1 = source.GetLowerBound(nonFixedDim1) let bound2 = source.GetLowerBound(nonFixedDim2) let bound3 = source.GetLowerBound(nonFixedDim3) - let start1, finish1 = ComputeSlice bound1 start1 finish1 (GetArray4DLength source nonFixedDim1) - let start2, finish2 = ComputeSlice bound2 start2 finish2 (GetArray4DLength source nonFixedDim2) - let start3, finish3 = ComputeSlice bound3 start3 finish3 (GetArray4DLength source nonFixedDim3) - let len1 = (finish1 - start1 + 1) - let len2 = (finish2 - start2 + 1) - let len3 = (finish3 - start3 + 1) + let _, len1 = ComputeSliceRange bound1 start1 finish1 (GetArray4DLength source nonFixedDim1) + let _, len2 = ComputeSliceRange bound2 start2 finish2 (GetArray4DLength source nonFixedDim2) + let _, len3 = ComputeSliceRange bound3 start3 finish3 (GetArray4DLength source nonFixedDim3) let dst = Array3DZeroCreate (max len1 0) (max len2 0) (max len3 0) let getArrayElem = @@ -6516,10 +6518,8 @@ namespace Microsoft.FSharp.Core let inline GetArraySlice4DFixedDouble (source: _[,,,]) start1 finish1 start2 finish2 index1 index2 nonFixedDim1 nonFixedDim2 = let bound1 = source.GetLowerBound(nonFixedDim1) let bound2 = source.GetLowerBound(nonFixedDim2) - let start1, finish1 = ComputeSlice bound1 start1 finish1 (GetArray4DLength source nonFixedDim1) - let start2, finish2 = ComputeSlice bound2 start2 finish2 (GetArray4DLength source nonFixedDim2) - let len1 = (finish1 - start1 + 1) - let len2 = (finish2 - start2 + 1) + let _, len1 = ComputeSliceRange bound1 start1 finish1 (GetArray4DLength source nonFixedDim1) + let _, len2 = ComputeSliceRange bound2 start2 finish2 (GetArray4DLength source nonFixedDim2) let dst = Array2DZeroCreate (max len1 0) (max len2 0) let getArrayElem = @@ -6557,8 +6557,7 @@ namespace Microsoft.FSharp.Core let inline GetArraySlice4DFixedTriple (source: _[,,,]) start1 finish1 index1 index2 index3 nonFixedDim1 = let bound1 = source.GetLowerBound(nonFixedDim1) - let start1, finish1 = ComputeSlice bound1 start1 finish1 (GetArray4DLength source nonFixedDim1) - let len1 = (finish1 - start1 + 1) + let _, len1 = ComputeSliceRange bound1 start1 finish1 (GetArray4DLength source nonFixedDim1) let dst = zeroCreate (max len1 0) let getArrayElem = match nonFixedDim1 with @@ -6702,8 +6701,7 @@ namespace Microsoft.FSharp.Core SetArraySlice4DFixedTriple target source index1 index2 index3 start4 finish4 3 let inline GetStringSlice (source: string) start finish = - let start, finish = ComputeSlice 0 start finish source.Length - let len = finish-start+1 + let start, len = ComputeSliceRange 0 start finish source.Length if len <= 0 then String.Empty else source.Substring(start, len) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/OperatorsModule1.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/OperatorsModule1.fs index 0032fc36032..456deee3a1c 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/OperatorsModule1.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/OperatorsModule1.fs @@ -93,6 +93,118 @@ type OperatorsModule1() = // null CheckThrowsNullRefException(fun () -> Operators.OperatorIntrinsics.GetStringSlice null param1 param2 |> ignore) + static member GetterSlicingOverflowCases() = + let hi, lo = Int32.MaxValue, Int32.MinValue + let a1 = [|1;2;3|] + let a2 = Array2D.zeroCreate 2 3 + let a3 = Array3D.zeroCreate 2 3 4 + let a4 = Array4D.zeroCreate 2 3 4 5 + let positive = Array2D.zeroCreateBased 3 5 2 3 + let negative = Array2D.zeroCreateBased -3 5 2 3 + let endpoint = Array.CreateInstance(typeof, [|1;2|], [|hi;0|]) :?> int[,] + let empty = Array.CreateInstance(typeof, [|0;2|], [|lo;0|]) :?> int[,] + let body name (args: obj[]) () = + let m = typeof.Assembly.GetType("Microsoft.FSharp.Core.Operators+OperatorIntrinsics").GetMethod(name) + Assert.NotNull m + m.MakeGenericMethod(typeof).Invoke(null, args) :?> Array + let cases: (string * int list * (unit -> Array)) list = + [ + "syntax 1D count two", [0], (fun () -> a1[hi..lo]) + "syntax 1D count three", [0], (fun () -> a1[hi..(lo + 1)]) + "syntax 2D axis 0", [0;3], (fun () -> a2[hi..lo, *]) + "syntax 2D axis 1", [2;0], (fun () -> a2[*, hi..lo]) + "syntax 3D axis 0", [0;3;4], (fun () -> a3[hi..lo, *, *]) + "syntax 3D axis 1", [2;0;4], (fun () -> a3[*, hi..lo, *]) + "syntax 3D axis 2", [2;3;0], (fun () -> a3[*, *, hi..lo]) + "syntax 4D axis 0", [0;3;4;5], (fun () -> a4[hi..lo, *, *, *]) + "syntax 4D axis 1", [2;0;4;5], (fun () -> a4[*, hi..lo, *, *]) + "syntax 4D axis 2", [2;3;0;5], (fun () -> a4[*, *, hi..lo, *]) + "syntax 4D axis 3", [2;3;4;0], (fun () -> a4[*, *, *, hi..lo]) + "syntax 2D fixed", [0], (fun () -> a2[0, hi..lo]) + "syntax 3D fixed single", [0;4], (fun () -> a3[0, hi..lo, *]) + "syntax 3D fixed double", [0], (fun () -> a3[0, 0, hi..lo]) + "syntax 4D fixed single", [0;4;5], (fun () -> a4[0, hi..lo, *, *]) + "syntax 4D fixed double", [0;5], (fun () -> a4[0, 0, hi..lo, *]) + "syntax 4D fixed triple", [0], (fun () -> a4[0, 0, 0, hi..lo]) + "syntax 2D fixed loop", [0], (fun () -> a2[0, 1..lo]) + "syntax 3D fixed loop", [0;4], (fun () -> a3[0, 1..lo, *]) + "syntax 4D fixed loop", [0;4;5], (fun () -> a4[0, 1..lo, *, *]) + "positive based", [0;3], (fun () -> positive[hi..lo, *]) + "negative based", [0;3], (fun () -> negative[hi..lo, *]) + "inclusive endpoint", [0;2], (fun () -> endpoint[..lo, *]) + "empty based explicit finish", [0;2], (fun () -> empty[hi..lo, *]) + "empty based omitted finish", [0;2], (fun () -> empty[hi.., *]) + "body 1D count two", [0], body "GetArraySlice" [|a1; Some hi; Some lo|] + "body 1D count three", [0], body "GetArraySlice" [|a1; Some hi; Some(lo + 1)|] + "body 2D", [0;3], body "GetArraySlice2D" [|a2; Some hi; Some lo; None; None|] + "body 3D", [0;3;4], body "GetArraySlice3D" [|a3; Some hi; Some lo; None; None; None; None|] + "body 4D", [0;3;4;5], body "GetArraySlice4D" [|a4; Some hi; Some lo; None; None; None; None; None; None|] + "body 2D fixed", [0], body "GetArraySlice2DFixed1" [|a2; 0; Some hi; Some lo|] + "body 3D fixed single", [0;4], body "GetArraySlice3DFixedSingle1" [|a3; 0; Some hi; Some lo; None; None|] + "body 3D fixed double", [0], body "GetArraySlice3DFixedDouble1" [|a3; 0; 0; Some hi; Some lo|] + "body 4D fixed single", [0;4;5], body "GetArraySlice4DFixedSingle1" [|a4; 0; Some hi; Some lo; None; None; None; None|] + "body 4D fixed double", [0;5], body "GetArraySlice4DFixedDouble1" [|a4; 0; 0; Some hi; Some lo; None; None|] + "body 4D fixed triple", [0], body "GetArraySlice4DFixedTriple4" [|a4; 0; 0; 0; Some hi; Some lo|] + "body 2D fixed loop", [0], body "GetArraySlice2DFixed1" [|a2; 0; Some 1; Some lo|] + "body 3D fixed loop", [0;4], body "GetArraySlice3DFixedSingle1" [|a3; 0; Some 1; Some lo; None; None|] + "body 4D fixed loop", [0;4;5], body "GetArraySlice4DFixedSingle1" [|a4; 0; Some 1; Some lo; None; None; None; None|] + ] + cases |> Seq.map (fun (name, shape, slice) -> [|box name; box shape; box slice|]) + + static member private CheckSliceShape(expected: int list, actual: Array) = + Assert.AreEqual(expected.Length, actual.Rank) + expected |> List.iteri (fun d length -> + Assert.AreEqual(length, actual.GetLength(d)) + Assert.AreEqual(0, actual.GetLowerBound(d))) + + [] + member _.GetterSlicingOverflowShape(_name: string, expected: int list, slice: unit -> Array) = + OperatorsModule1.CheckSliceShape(expected, slice()) + + [] + [] + [] + [] + [] + member _.GetterSlicingOverflowString(start: int, callableBody: bool) = + let actual = + if callableBody then + let m = typeof.Assembly.GetType("Microsoft.FSharp.Core.Operators+OperatorIntrinsics").GetMethod("GetStringSlice") + Assert.NotNull m + m.Invoke(null, [|"hello"; Some start; Some Int32.MinValue|]) :?> string + else + "hello"[start..Int32.MinValue] + Assert.AreEqual(String.Empty, actual) + + [] + member _.GetterSlicingOverflowFinishBeforeUpperEndpoint() = + let start = Int32.MaxValue - 1 + let source = Array.CreateInstance(typeof, [|2;2|], [|start;0|]) :?> int[,] + source[start, 0] <- 42 + source[start, 1] <- 43 + let actual = source[start..start, *] + OperatorsModule1.CheckSliceShape([1;2], actual) + Assert.AreEqual(42, actual[0, 0]) + Assert.AreEqual(43, actual[0, 1]) + + [] + member _.GetterSlicingControls() = + let hi, lo = Int32.MaxValue, Int32.MinValue + Assert.AreEqual([], [1..5][3..lo]) + OperatorsModule1.CheckSliceShape([0], [|1;2;3|][3..(lo + 10)]) + let source = Array2D.initBased -3 5 2 3 (fun i j -> 100 * i + j) + let actual = source[-3..-2, *] + OperatorsModule1.CheckSliceShape([2;3], actual) + for i in 0..1 do + for j in 0..2 do + Assert.AreEqual(source[i - 3, j + 5], actual[i, j]) + let a2 = Array2D.zeroCreate 2 3 + OperatorsModule1.CheckSliceShape([0], a2[2, 1..0]) + CheckThrowsIndexOutRangException(fun () -> a2[2, 0..0] |> ignore) + CheckThrowsNullRefException(fun () -> Operators.OperatorIntrinsics.GetArraySlice (null: int[]) (Some hi) (Some lo) |> ignore) + CheckThrowsNullRefException(fun () -> Operators.OperatorIntrinsics.GetArraySlice2DFixed1 (null: int[,]) 0 (Some hi) (Some lo) |> ignore) + CheckThrowsNullRefException(fun () -> Operators.OperatorIntrinsics.GetStringSlice null (Some hi) (Some lo) |> ignore) + [] member _.OptimizedRangesSetArraySlice() = let param1 = Some(1) From eae0637ae6af687848fe28435725dfbea651904c Mon Sep 17 00:00:00 2001 From: Copilot Date: Tue, 15 Sep 2026 16:38:14 +0200 Subject: [PATCH 3/5] Add release notes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6fa70800-9879-48dc-b7c3-a195759cf4dd --- docs/release-notes/.FSharp.Core/11.0.100.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Core/11.0.100.md b/docs/release-notes/.FSharp.Core/11.0.100.md index 7dfc227a087..5ed1fcffee3 100644 --- a/docs/release-notes/.FSharp.Core/11.0.100.md +++ b/docs/release-notes/.FSharp.Core/11.0.100.md @@ -5,7 +5,7 @@ * Fix `Array.exists2` documentation examples to use equal-length arrays; the previous examples would throw `ArgumentException` at runtime instead of returning the documented `false`/`true` values. ([PR #19672](https://github.com/dotnet/fsharp/pull/19672)) * Move `Async.StartChild` to the "Starting Async Computations" docs category alongside `Async.StartChildAsTask`. ([Issue #19667](https://github.com/dotnet/fsharp/issues/19667)) * Add `InlineIfLambda` to `Array.init` ([PR #19869](https://github.com/dotnet/fsharp/pull/19869)) -* Fix array and string slices with extreme reversed bounds to return correctly shaped empty results. ([Issue #20530](https://github.com/dotnet/fsharp/issues/20530)) +* Fix array and string slices with extreme reversed bounds to return correctly shaped empty results. ([Issue #20530](https://github.com/dotnet/fsharp/issues/20530), [PR #20557](https://github.com/dotnet/fsharp/pull/20557)) * Fix printf handling of -0.0 (negative zero) values for float, float32, and decimal values ([Issue #15557](https://github.com/dotnet/fsharp/issues/15557) and [Issue #15558](https://github.com/dotnet/fsharp/issues/15558), [PR #18147](https://github.com/dotnet/fsharp/pull/18147)) From 296a4aa1285ae8b84763016f4d2735505063389d Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 16 Sep 2026 00:25:11 +0200 Subject: [PATCH 4/5] Guard Desktop-unsupported slicing endpoint fixtures Recover the test-only delta from 1afa05af5860496aa21e5611581406b0f8a0a634. Keep all supported Desktop rows and preserve the getter implementation. Both compressed Release SDK configurations pass Desktop/CoreCLR shape 38/39, getters 43/45, siblings 125/127, and full Core suites 6255/6337 with five existing skips each. Exact VS composite configurations remain pending on compatible infrastructure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8fdec442-2296-4dcd-b96f-f499a8485cdd --- .../FSharp.Core/OperatorsModule1.fs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/OperatorsModule1.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/OperatorsModule1.fs index 456deee3a1c..24c9817201f 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/OperatorsModule1.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/OperatorsModule1.fs @@ -101,7 +101,6 @@ type OperatorsModule1() = let a4 = Array4D.zeroCreate 2 3 4 5 let positive = Array2D.zeroCreateBased 3 5 2 3 let negative = Array2D.zeroCreateBased -3 5 2 3 - let endpoint = Array.CreateInstance(typeof, [|1;2|], [|hi;0|]) :?> int[,] let empty = Array.CreateInstance(typeof, [|0;2|], [|lo;0|]) :?> int[,] let body name (args: obj[]) () = let m = typeof.Assembly.GetType("Microsoft.FSharp.Core.Operators+OperatorIntrinsics").GetMethod(name) @@ -131,7 +130,11 @@ type OperatorsModule1() = "syntax 4D fixed loop", [0;4;5], (fun () -> a4[0, 1..lo, *, *]) "positive based", [0;3], (fun () -> positive[hi..lo, *]) "negative based", [0;3], (fun () -> negative[hi..lo, *]) - "inclusive endpoint", [0;2], (fun () -> endpoint[..lo, *]) +#if NETCOREAPP + "inclusive endpoint", [0;2], (fun () -> + let endpoint = Array.CreateInstance(typeof, [|1;2|], [|hi;0|]) :?> int[,] + endpoint[..lo, *]) +#endif "empty based explicit finish", [0;2], (fun () -> empty[hi..lo, *]) "empty based omitted finish", [0;2], (fun () -> empty[hi.., *]) "body 1D count two", [0], body "GetArraySlice" [|a1; Some hi; Some lo|] @@ -176,6 +179,7 @@ type OperatorsModule1() = "hello"[start..Int32.MinValue] Assert.AreEqual(String.Empty, actual) +#if NETCOREAPP [] member _.GetterSlicingOverflowFinishBeforeUpperEndpoint() = let start = Int32.MaxValue - 1 @@ -186,6 +190,7 @@ type OperatorsModule1() = OperatorsModule1.CheckSliceShape([1;2], actual) Assert.AreEqual(42, actual[0, 0]) Assert.AreEqual(43, actual[0, 1]) +#endif [] member _.GetterSlicingControls() = From 2c371f36a9ee3e10b4f1fdf1753c2c14b775c2a2 Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 16 Sep 2026 00:25:11 +0200 Subject: [PATCH 5/5] Remove archived Ralph planning documents from the PR Keep working copies for the runner. Preserve plans and validation evidence outside the worktree. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8fdec442-2296-4dcd-b96f-f499a8485cdd --- .tools/ralph/BACKLOG.md | 176 -------- .../sprints/01_Fix_Getter_Slice_Overflow.md | 386 ------------------ 2 files changed, 562 deletions(-) delete mode 100644 .tools/ralph/BACKLOG.md delete mode 100644 .tools/ralph/sprints/01_Fix_Getter_Slice_Overflow.md diff --git a/.tools/ralph/BACKLOG.md b/.tools/ralph/BACKLOG.md deleted file mode 100644 index 6b3039c6a2c..00000000000 --- a/.tools/ralph/BACKLOG.md +++ /dev/null @@ -1,176 +0,0 @@ -# BACKLOG - -## Original Request - -Process issue https://github.com/dotnet/fsharp/issues/20530 using TDD. - -Use minimal, surgical changes. Validate locally before finishing. Do not push. Only commit. - -### ISSUE REQUEST -The requirements above override conflicting instructions in the issue request. -Fix issue https://github.com/dotnet/fsharp/issues/20530. - -Make the smallest clean, correct, complete fix. Keep it minimal and surgical. Quality matters more than time. Take all the time needed. Smaller is better, but not at the cost of correctness. Preserve progress and evidence across execution windows rather than truncate the work. - -**Verified root cause.** At main `b5c530ed6bc42937de6363e3dcc104ebb833893d`, getter slices compute inclusive lengths with unchecked arithmetic. Reversed bounds can wrap to a positive count. Fixed-index getters can also iterate incorrectly when `len = Int32.MinValue` makes `len - 1` wrap, despite an empty allocation. Legal based-array endpoints expose a directly coupled overflow in `ComputeSlice`'s exclusive upper-bound arithmetic. Current-main source execution and both Core targets reproduce the defect. Each Core target has 17 safe failing expected-empty assertions. Ten source-syntax controls pass. - -**Surgical candidate.** Keep changes in `src/FSharp.Core/prim-types.fs`. Add one implementation-only getter normalizer near `ComputeSlice` at `6265-6275`, returning `(start, count)`. Handle a zero-length dimension before deriving an upper bound. For a nonempty legal dimension, use `bound + (length - 1)`, then compare the clipped endpoints before subtraction. Use it across the eleven getter implementations, covering twenty-one dimension counts between `6277` and `6708`. This includes all full-rank and fixed-index getter families and string slicing. - -The proposed helper is not yet compiled or GREEN. Its arithmetic is supported by an interval-intersection proof and 14,504 allocation-free boundary-model cases. For ordered clipped endpoints, the count cannot exceed the source dimension length. Verify the actual inline implementation and emitted consumer behavior, not just the model. - -Reuse existing allocation and copy helpers at `prim-types.fs:799-922`. The helper's returned start must equal today's `ComputeSlice` low exactly. Preserve existing element-access expressions while replacing length calculations. Keep `ComputeSlice` for fixed setters because changing it would alter setter semantics. Do not repair unrelated fixed-getter source offsets, setter arithmetic, reverse-index translation, compiler syntax, or public APIs. These adjacent issues were observed and are explicitly outside this fix. Preserve inclusive bounds, retained rank and shape, zero-based results, normal null exceptions, and current fixed-index validation timing. - -**RED-first test plan.** Use the existing Core unit-test project, preferably local cases near `tests/FSharp.Core.UnitTests/FSharp.Core/OperatorsModule1.fs:43-95` and existing slicing tests. Use compact typed thunks, data rows, or intrinsic entry points instead of a new reflection framework. Assert correct empty results, not exceptions from the broken implementation. - -**Allocation safety:** do not run the original array `[3..Int32.MinValue]` as an OOM experiment. `"hello"[3..Int32.MinValue]` is safe and preserves the original symptom. Array bounds `Int32.MaxValue..Int32.MinValue` produce a broken count of only two, giving deterministic safe RED. Keep all actual arrays tiny. - -| Scenario | Required assertion | -|---|---| -| 1. Original string `3..MinValue`, plus `MaxValue..MinValue` | Empty string without exception. | -| 2. 1D arrays with `MaxValue..MinValue` and `MaxValue..(MinValue+1)` | Empty arrays. Broken counts are two and three, not huge allocations. | -| 3. Full 2D, 3D, and 4D getters | Rotate the extreme reversed range through retained axes. Assert rank, every dimension length, and zero result lower bounds. Empty one axis, not all axes. | -| 4. Fixed-index 2D-4D getters | Cover the six underlying fixed getter implementations. Assert reduced rank and the lengths of other retained dimensions. Some current failures return wrong nonempty shapes rather than throw. | -| 5. Fixed-loop boundary `1..MinValue` | Empty result with exact shape in 2D, 3D, and 4D. Current allocation can be empty while the loop still enters. | -| 6. Positive and negative source lower bounds | Extreme reversed retained ranges yield correctly shaped empty slices. Separately preserve valid negative absolute indices. | -| 7. Legal upper-bound endpoint | A one-element dimension based at MaxValue, sliced through MinValue, must produce an empty retained dimension. Include a tiny dimension ending at MaxValue with a finish inside it. Do not compute an overflowing exclusive bound. | -| 8. Empty dimension based at MinValue, requested start MaxValue | Correct empty retained dimension, no element access, and unchanged lengths of other axes. Do not derive its upper bound before noticing zero length. | -| 9. Existing negative controls | Reuse list, nearby nonoverflow, omitted-bound, ordinary clipping, empty/copy identity, null, invalid fixed-index timing, normal setter, and normal reverse-slice tests. Keep these separate from RED claims. | - -Rows 1-6 provide the primary and five meaningful variants. Rows 7-8 directly protect the arithmetic proof at legal source boundaries. Row 9 uses existing coverage wherever possible. Do not create a Cartesian product of all types, axes, flags, or platforms. Test the six fixed implementations without duplicating every wrapper fixture. - -First record RED with correct assertions against current main. Apply the smallest complete getter change and get the same cases GREEN without weakening shape checks or changing expected exceptions to match the bug. Recompile consumers against the new Core: existing consumers can retain old public-inline arithmetic. Exercise both ordinary F# slicing and callable intrinsic bodies where needed. Do not promise a Core DLL replacement alone repairs previously compiled consumers. - -Run the focused sibling slicing selection and Core suite. The starting baseline passed 33 tests selected by `*SlicingOutOfBounds` and `*Fixed*`. Use the repository's composite Core build for the implementation. Format only changed F# files. Invoke the expert-review skill on the final work. Remove noise, duplicate setup, and unnecessary helpers. Leave a clean, compact suite and concise release note. Avoid setter changes, public surface changes, baseline churn, and unrelated cleanup. - -Sources: [issue](https://github.com/dotnet/fsharp/issues/20530), [current slicing intrinsics](https://github.com/dotnet/fsharp/blob/b5c530ed6bc42937de6363e3dcc104ebb833893d/src/FSharp.Core/prim-types.fs#L6265-L6708), [FS-1077 tolerant slicing](https://github.com/fsharp/fslang-design/blob/7e3f0db7dcf1daa9486c7c87d3f5a398d460f56b/FSharp-5.0/FS-1077-tolerant-slicing.md), [related design work](https://github.com/fsharp/fslang-design/pull/849). This fix does not depend on that draft RFC. - -## Analysis - -### Planning scope and verified repository facts - -This is an architecture handoff, not the implementation. The deliverables are this backlog and one self-contained sprint. -The sprint includes tests, the complete getter fix, consumer validation, review, release notes, and a local commit. -Splitting RED tests into a separate sprint would leave an intentionally failing unit of work. -Splitting getter families would leave one shared arithmetic defect only partly fixed. - -The worktree is `Q:\fsharp-worktrees\issue-875`, on branch `fix/issue-20530`. -Its initial HEAD is exactly `b5c530ed6bc42937de6363e3dcc104ebb833893d`. -The tracked worktree was clean before planning. -The existing `.tools\ralph\ralph.log` is runner-owned and must remain untouched. -`.gitignore` ignores `.tools`, so commit the two requested planning files with explicit paths and `git add -f`. -Do not force-add the directory, logs, or later evidence. - -The issue is open and has no comments at planning time. -The pinned FS-1077 text defines inclusive, clipped getter slices, with empty results for disjoint bounds. -No compiler feature gate or dependency on fsharp/fslang-design#849 is required. - -Direct inspection confirms the three coupled arithmetic problems: - -1. `finish - start + 1` can wrap before allocation helpers see the count. -2. Fixed getters clamp allocation dimensions but loop using the original count. -3. `ComputeSlice` uses `bound + length` for its comparison, which overflows at legal inclusive endpoints. - -The existing `ComputeSlice` low is `max(bound, requestedStart)`, with omitted start mapped to `bound`. -Keep that exact low, including empty intervals and valid negative absolute indices. -The new getter-only helper must not change fixed setters, which still call `ComputeSlice`. -Full-rank allocation/copy helpers already exist at `prim-types.fs:799-922`. - -The implementation inventory contains eleven bodies and twenty-one dimension calculations: - -| Getter body | Dimension counts | -|---|---:| -| `GetArraySlice` | 1 | -| `GetArraySlice2D` | 2 | -| `GetArraySlice2DFixed` | 1 | -| `GetArraySlice3D` | 3 | -| `GetArraySlice3DFixedSingle` | 2 | -| `GetArraySlice3DFixedDouble` | 1 | -| `GetArraySlice4D` | 4 | -| `GetArraySlice4DFixedSingle` | 3 | -| `GetArraySlice4DFixedDouble` | 2 | -| `GetArraySlice4DFixedTriple` | 1 | -| `GetStringSlice` | 1 | - -The six fixed bodies are implementation-only. Their numbered wrappers are public inline functions in `prim-types.fsi`. -Use those wrappers to reach the six bodies without exposing new APIs. -Some fixed bodies omit retained source offsets. Preserve those expressions exactly, as the request explicitly excludes those defects. - -### Validation facts and limitations - -The prior 17 failing assertions per Core target, ten passing syntax controls, 33 sibling passes, and 14,504 model cases are user-supplied evidence. -They were not rerun during architecture work. Do not label them as this sprint's RED or GREEN. -New tests must independently record failures against unchanged product source. -There is no required new-test count of 17. Cover every requested behavior without duplicating wrappers. - -The current Core project declares **three** non-Proto targets: `netstandard2.0`, `netstandard2.1`, and the shipped-net target. -`eng\TargetFrameworks.props` currently pins the shipped-net target to `net10.0` and the product/test runtime to `net11.0`. -The unit-test project normally references the shipped-net Core on CoreCLR. -It also builds `netstandard2.1` for a separate surface-area test. -Therefore, a default CoreCLR run alone does not prove both netstandard implementations. -The sprint requires focused consumer execution against both netstandard assemblies and the default shipped-net assembly. -This is a Core-target check, not a Cartesian platform matrix. - -`global.json` selects SDK `11.0.100-rc.1.26420.103` and Microsoft.Testing.Platform. -The planning-time `dotnet msbuild ... -getProperty:...` probe could not start because the pinned SDK is unavailable. -There is no worktree `.dotnet\dotnet.exe` or built Core output. -Use the repository's Windows SDK acquisition wrapper before implementation validation. -No product build or test was attempted during planning. -The existing `build.cmd` invokes `eng\Build.ps1 -restore -build`. -Its `-noVisualStudio` route uses the composite `FSharp.slnx`, including Core and the Core unit-test project. -Direct test-assembly execution avoids ambiguity between MTP and older VSTest filter syntax. - -Local planning validation passed: required headings/frontmatter, 19 plain Definition of Done criteria, Markdown table widths, and ten principal repository paths. -The getter inventory was checked against the actual source: eleven bodies with twenty-one `ComputeSlice` calls. -The sprint was read independently for scenario coverage and does not require this backlog. -Implementation commands remain future work. Their inclusion is not a claim that the compiler, candidate helper, or tests have run. - -The GitHub `VNEXT` variable is `11.0.100`. -The release-note destination currently exists at `docs\release-notes\.FSharp.Core\11.0.100.md`. -The no-push/no-PR instruction means the local note must use the real issue link, not a fabricated PR number. - -The shared issue-queue database was not available at either configured user path. -No remote triage job or daily-monitor dispatch was requested or created. -This backlog preserves the request locally instead. - -## Approach - -Use one end-to-end sprint, with no prerequisite sprint: - -1. Bootstrap the missing SDK through repository tooling, build unchanged Core, and record sibling controls. -2. Add compact, separately discoverable expected-empty regression cases in `OperatorsModule1.fs`. -3. Record safe RED before touching `prim-types.fs`, including wrong-shape returns and fixed-loop failures. -4. Add one implementation-only inline `(start, count)` normalizer. -5. Wire all eleven getters and all twenty-one retained-dimension counts without changing source element access. -6. Rebuild the composite and recompile consumers. Run identical tests GREEN against each relevant Core target. -7. Run focused sibling slicing tests, the full Core suite, and unchanged surface-area checks. -8. Format only changed F# files, obtain the requested expert review, resolve findings, and rerun affected validation. -9. Add one concise release note and commit only the intended implementation files. Never push or create a PR. - -For a nonempty legal dimension, `upper = bound + (length - 1)` is representable. -If the clipped high is below the unchanged low, return count zero before subtraction. -Otherwise, `bound <= low <= high <= upper`, so `1 <= high - low + 1 <= length`. -For an empty source dimension, return count zero before calculating an upper bound. -This proof supports the candidate. It does not replace compilation or consumer execution. - -Preserve raw RED/GREEN logs, exact commands, Core paths/hashes, and review outcomes in ignored issue-specific evidence. -Include a short verification summary in the implementation commit message so the result remains durable beyond the runner session. -Do not commit generated logs, temporary projects, or copied Core assemblies. - -### Final verification checklist - -- The sprint file has the required frontmatter, sections, and plain dash-list Definition of Done. -- An implementer can complete the work from the sprint alone, without this backlog or another sprint. -- All nine scenario rows, six fixed bodies, eleven getter bodies, and twenty-one dimension counts are covered. -- Required RED assertions remain unchanged in GREEN, with rank, all lengths, and all lower bounds checked. -- Both netstandard Core targets and the default shipped-net target have freshly compiled consumer evidence. -- Source syntax and callable intrinsic bodies are distinguished in the evidence. -- No setter, element-offset, compiler, public-signature, baseline, or unrelated formatting change is accepted. -- Composite build, focused controls, full Core suite, formatting, and expert review are recorded. -- The release note exists and does not promise repair of previously compiled inline consumers. -- Only intended files are committed. Nothing is pushed, and no GitHub write operation occurs. - -## Sprint Overview - -| # | Name | Purpose | -|---|---|---| -| 01 | Fix Getter Slice Overflow | Complete RED-to-GREEN fix for every getter, including shape/boundary regressions, consumer validation, review, release note, and local commit. | diff --git a/.tools/ralph/sprints/01_Fix_Getter_Slice_Overflow.md b/.tools/ralph/sprints/01_Fix_Getter_Slice_Overflow.md deleted file mode 100644 index 683d14b336b..00000000000 --- a/.tools/ralph/sprints/01_Fix_Getter_Slice_Overflow.md +++ /dev/null @@ -1,386 +0,0 @@ ---- ---- - -# Sprint: Fix getter slice overflow with RED-first regressions - -## Context - WHY this sprint exists - -Fix https://github.com/dotnet/fsharp/issues/20530 in `Q:\fsharp-worktrees\issue-875`. -This sprint is the complete implementation unit. It has no prerequisite sprint. -You receive only this file. All implementation boundaries and acceptance requirements are below. -Use TDD, validate locally, and commit the result. Do not push, open a PR, or post GitHub comments. - -The planned base is `b5c530ed6bc42937de6363e3dcc104ebb833893d`, on branch `fix/issue-20530`. -Inspect the actual HEAD and working tree before editing. Preserve other agents' work and runner-owned files. -Do not reset or replace the branch to match this document. - -F# tolerant getter slices use inclusive bounds. -After clipping to the source dimension, disjoint bounds must return an empty result. -The current getters calculate `finish - start + 1` with unchecked `int` arithmetic. -Extreme reversed bounds can wrap to a positive allocation count. -Fixed getters also loop with an unclamped count. A count of `Int32.MinValue` makes `len - 1` wrap. -The shared `ComputeSlice` additionally overflows at legal based-array endpoints because it calculates the exclusive bound `bound + length`. - -The request reports 17 safe failing assertions per previously tested Core target, ten passing syntax controls, and 33 passing sibling tests. -Those results are background evidence, not this sprint's RED record. -The candidate helper has not been compiled or proved GREEN. -An interval proof and 14,504 model cases support its arithmetic but do not validate inline consumer behavior. - -Reference semantics: [FS-1077 tolerant slicing](https://github.com/fsharp/fslang-design/blob/7e3f0db7dcf1daa9486c7c87d3f5a398d460f56b/FSharp-5.0/FS-1077-tolerant-slicing.md). -The fix does not depend on fsharp/fslang-design#849. - -## Description - WHAT to implement with DETAILED guidance - -### Files and repository rules - -All relative paths below start at `Q:\fsharp-worktrees\issue-875`. - -| Path | Action | -|---|---| -| `src\FSharp.Core\prim-types.fs` | Add one implementation-only getter normalizer and replace getter count calculations. | -| `tests\FSharp.Core.UnitTests\FSharp.Core\OperatorsModule1.fs` | Add compact regression cases beside the existing intrinsic slicing tests, currently near lines 43-95. | -| `docs\release-notes\.FSharp.Core\11.0.100.md` | Add one concise `Fixed` entry after validation. Recheck the current `VNEXT` value. | - -Read `.github\instructions\FSharpCore.instructions.md` and its linked `docs\fsharp-core-notes.md` before changing Core. -Follow `.github\instructions\NoBloat.instructions.md` for compact code and test setup. -Read any additional instructions that apply to the actual files you touch. -Use `hypothesis-driven-debugging` for RED/failure investigation. -Use `binlog-analysis` if a build fails. Diagnose the recorded binlog and repair the cause before continuing. -Do not edit `eng\common` to repair local setup. Those files are maintained by Arcade. - -The existing project is `tests\FSharp.Core.UnitTests\FSharp.Core.UnitTests.fsproj`. -The fixture is `FSharp.Core.UnitTests.Operators.OperatorsModule1`. -It uses xUnit and `FSharp.Core.UnitTests.LibraryTestFx`, including `Assert.AreEqual` and `CheckThrowsNullRefException`. -Follow `OptimizedRangesGetArraySlice`, `OptimizedRangesGetArraySlice2D`, and `OptimizedRangesGetStringSlice`. -No new test project, project-file entry, package, public API, or compiler change is expected. - -### 1. Establish the local baseline and durable evidence - -Create an ignored evidence directory at `.tools\ralph\evidence\issue-20530`. -Preserve commands, exit codes, test discovery/counts, failures, binlogs, and Core assembly identities there. -Use distinct RED, GREEN, and control log names. Do not overwrite RED evidence during later runs. -Record the source commit and whether `prim-types.fs` was unchanged for each RED run. -Record unresolved work before an execution window ends. Resume from that evidence instead of skipping validation. -Do not commit logs, temporary consumers, package caches, or build output. - -The planning-time SDK probe failed before MSBuild could start. -`global.json` currently requires `11.0.100-rc.1.26420.103`. -If that SDK remains missing, run the existing acquisition wrapper: - -```powershell -Set-Location 'Q:\fsharp-worktrees\issue-875' -& .\eng\common\dotnet.ps1 --info -``` - -Use the acquired SDK consistently. If it is installed under `.dotnet`, invoke `.\.dotnet\dotnet.exe`. -Below, `dotnet` means that matching SDK, not an unrelated SDK or FSI installation. -Read target frameworks rather than inferring them from the issue's earlier two-target evidence: - -```powershell -$env:BUILDING_USING_DOTNET = 'true' -dotnet msbuild src\FSharp.Core\FSharp.Core.fsproj -nologo -getProperty:TargetFrameworks -dotnet msbuild tests\FSharp.Core.UnitTests\FSharp.Core.UnitTests.fsproj -nologo -getProperty:TargetFrameworks,FSharpCoreShippedNetTargetFramework -``` - -At the planned base, Core targets `netstandard2.0`, `netstandard2.1`, and `net10.0`. -The product/CoreCLR test runtime is `net11.0`. CoreCLR unit tests normally reference the shipped `net10.0` Core. -Core assembly target and test-host runtime are different dimensions. -The test project also builds `netstandard2.1` for a surface-area test. Building it alone does not execute its getters. - -Use the repository composite build for Core and the compiler: - -```powershell -.\build.cmd -c Debug -noVisualStudio -``` - -`build.cmd` delegates to `eng\Build.ps1 -restore -build`. The no-Visual-Studio route builds `FSharp.slnx`. -Stop on a nonzero exit code and retain its binlog. -Do not replace the composite with a standalone Core build as the final implementation check. -If bootstrap contamination is diagnosed, preserve evidence before cleaning only the worktree's resolved `artifacts` output and rebuilding. - -After a successful build, run the existing sibling selection against unchanged Core: - -```powershell -dotnet exec artifacts\bin\FSharp.Core.UnitTests\Debug\net11.0\FSharp.Core.UnitTests.dll --filter-method "*SlicingOutOfBounds" --filter-method "*Fixed*" -``` - -Substitute the evaluated product TFM if it differs. -The request's starting result was 33 passing tests for this selection. -Record the actual discovery and outcomes. Explain any difference rather than claiming the old count. -Use the executable's `--help` if runner options differ. Never accept zero discovered tests. -The repository uses xUnit v3/Microsoft.Testing.Platform, not a VSTest filter expression. - -### 2. Add correct, allocation-safe tests and record RED - -Add the tests before editing `prim-types.fs`. -Use separate theory rows or facts so one exception does not prevent all other cases from executing. -Prefer typed `unit -> System.Array` thunks and a small shape assertion over reflection-driven test infrastructure. -A single local shape helper can check `Rank`, every `GetLength(d)`, and every `GetLowerBound(d)`. -Do not assert only total `Length` or use an empty assertion that cannot distinguish `[0;3]` from `[0;0]`. -Use `Assert.AreEqual` or established xUnit assertions. Include a useful case identifier in theory data. - -Use `hi = Int32.MaxValue` and `lo = Int32.MinValue`. -Keep sources tiny, for example dimensions `[2;3]`, `[2;3;4]`, and `[2;3;4;5]`. -Use distinct dimension lengths so preserved axes are observable. -Set valid fixed indices, normally zero, unless the case specifically checks validation timing. - -**Allocation safety:** never run an array getter with `3..lo`. -That reproducer can request a huge allocation before the fix. -The string `"hello"[3..lo]` is safe. -For arrays, `hi..lo` wraps to count two and `hi..(lo + 1)` wraps to count three. -The fixed-loop case `1..lo` allocates an empty result before the broken loop enters. -Do not add stress allocations or a Cartesian product of element types, wrappers, bounds, and platforms. - -Implement these scenario groups: - -| Group | Required cases and assertions | -|---|---| -| Original strings | `"hello"[3..lo]` and `"hello"[hi..lo]` both equal `String.Empty`, with no exception. | -| One-dimensional arrays | `[\|1;2;3\|][hi..lo]` and `[\|1;2;3\|][hi..(lo + 1)]` both have rank 1, length 0, and lower bound 0. | -| Full-rank getters | Rotate `hi..lo` through all axes. Other bounds are omitted. Check 2D shapes `[0;3]`, `[2;0]`; 3D shapes `[0;3;4]`, `[2;0;4]`, `[2;3;0]`; and 4D shapes `[0;3;4;5]`, `[2;0;4;5]`, `[2;3;0;5]`, `[2;3;4;0]`. Only one axis becomes empty. | -| Fixed-index getters | Cover each of the six implementation bodies using the representative wrapper cases below. Check reduced rank and all retained lengths. Wrong nonempty shapes are failures even if the call does not throw. | -| Fixed-loop boundary | Repeat a representative 2D, 3D, and 4D fixed case with `1..lo`. Keep other retained axes nonempty and check exact shapes. The call must return without source element access. | -| Based arrays | Use tiny positive-based and negative-based sources. `hi..lo` on a retained axis must produce a zero-based result with only that axis empty. Separately assert values for an ordinary valid negative absolute-index slice. | -| Inclusive upper endpoint | Use lengths `[1;2]` with lower bounds `[hi;0]`. A first-axis slice through `lo` must have shape `[0;2]`. Also use lengths `[2;2]` and bounds `[hi - 1;0]`; slice first-axis `hi - 1 .. hi - 1` and assert shape `[1;2]` and copied values. Its source ends at `hi`, but the finish lies before that endpoint. | -| Empty source endpoint | Use lengths `[0;2]` with bounds `[lo;0]`. Request first-axis start `hi`, first with finish `lo`, then with omitted finish. Assert shape `[0;2]`, rank 2, and zero lower bounds, without element access. | -| Negative controls | Reuse existing list, nearby nonoverflow, omitted-bound, clipping, empty/copy identity, null, setter, and reverse-slice coverage. Preserve fixed-index validation timing as described below. Do not count these controls as RED regressions. | - -Representative fixed cases, using the source dimensions above: - -| Underlying body | Public wrapper / ordinary syntax | Expected dimensions | -|---|---|---| -| `GetArraySlice2DFixed` | `GetArraySlice2DFixed1` / `a2[0, hi..lo]` | `[0]` | -| `GetArraySlice3DFixedSingle` | `GetArraySlice3DFixedSingle1` / `a3[0, hi..lo, *]` | `[0;4]` | -| `GetArraySlice3DFixedDouble` | `GetArraySlice3DFixedDouble1` / `a3[0, 0, hi..lo]` | `[0]` | -| `GetArraySlice4DFixedSingle` | `GetArraySlice4DFixedSingle1` / `a4[0, hi..lo, *, *]` | `[0;4;5]` | -| `GetArraySlice4DFixedDouble` | `GetArraySlice4DFixedDouble1` / `a4[0, 0, hi..lo, *]` | `[0;5]` | -| `GetArraySlice4DFixedTriple` | `GetArraySlice4DFixedTriple4` / `a4[0, 0, 0, hi..lo]` | `[0]` | - -The generic fixed bodies are not exposed in `prim-types.fsi`. -Call their numbered wrappers through `Operators.OperatorIntrinsics`, following the existing tests. -Do not add declarations to expose the helper or these internal bodies. -One representative wrapper per body is enough. Existing tests exercise the other wrappers. - -For based fixtures, use `Array2D.initBased` or `Array.CreateInstance(typeof, lengths, lowerBounds) :?> int[,]`. -For example, lower bounds `[-3;5]`, lengths `[2;3]`, and values `100 * i + j` support an ordinary `[-3..-2, *]` control. -Check its `[2;3]` result, zero lower bounds, and values at the corresponding absolute source indices. -Use full-rank or known-correct 2D paths for nonempty based controls. -Do not accidentally turn these tests into a repair of the excluded fixed-getter offset defects. - -Existing control locations, all under `tests\FSharp.Core.UnitTests\FSharp.Core`: - -| File | Existing coverage to reuse | -|---|---| -| `OperatorsModule1.fs` | Intrinsic array/string getters, normal null exception, and 1D-4D setters. | -| `Microsoft.FSharp.Collections\ArrayModule.fs` | `SlicingOutOfBounds`, omitted bounds, empty arrays, fresh nonempty copies, and referentially equivalent empty slices. | -| `Microsoft.FSharp.Collections\Array2Module.fs` | `SlicingBoundedStartEnd`, `SlicingOutOfBounds`, `SlicingMutation`, and ordinary reverse slicing. | -| `Microsoft.FSharp.Collections\Array3Module.fs` | Full slicing, `SlicingSingleFixed*`, `SlicingDoubleFixed*`, and reverse slicing. | -| `Microsoft.FSharp.Collections\Array4Module.fs` | Full slicing, `SlicingSingleFixed*`, `SlicingDoubleFixed*`, `SlicingTripleFixed*`, and reverse slicing. | -| `Microsoft.FSharp.Collections\StringModule.fs` | Bounded/unbounded, empty, out-of-bounds, and reverse string slicing. | -| `Microsoft.FSharp.Collections\ListType.fs` | Correct list slicing and out-of-bounds behavior. | - -If missing, add compact controls for `[1..5][3..lo] = []` and the safe nearby array range `3..(lo + 10)`. -Also preserve this timing with a tiny 2D source: invalid fixed index plus ordinary empty retained range `1..0` returns empty. -The same invalid fixed index with a nonempty retained range `0..0` still raises `IndexOutOfRangeException`. -Null array/string sources must still raise the existing null exception, even for an empty requested slice. -Do not introduce eager fixed-index validation or an early return before reading source dimensions. - -Name regression methods consistently, for example `GetterSlicingOverflow*`. -Rebuild the test consumer against unchanged product source and run that selection. -Before changing product source, use the target-selection procedure in step 4 to capture RED for both netstandard targets and the shipped-net target. -Record failures from correct expected-empty and shape assertions, not tests expecting today's exceptions. -Some source-syntax paths can pass because of compiler lowering or optimization. -Record them as passing controls and obtain RED through the relevant public intrinsic entry point or callable body. -Do not claim that a passing syntax example proves a failing intrinsic is covered. - -### 3. Implement one getter-only normalizer - -Edit only getter logic in `src\FSharp.Core\prim-types.fs`, currently around lines 6265-6708. -Leave the existing `ComputeSlice` definition unchanged for fixed setters. -Add one implementation-only inline helper nearby, for example `ComputeSliceRange`. -Match the existing helper's visibility pattern: an implementation binding absent from `prim-types.fsi`. -Compile it before assuming that it is accessible correctly through public inline optimization data. - -Candidate structure, not prevalidated production code: - -```fsharp -let inline ComputeSliceRange bound start finish length = - let low = - match start with - | Some n when n >= bound -> n - | _ -> bound - - let count = - if length = 0 then - 0 - else - let upper = bound + (length - 1) - let high = - match finish with - | Some n when n < upper -> n - | _ -> upper - - if high < low then 0 else high - low + 1 - - low, count -``` - -The returned low must exactly match today's `ComputeSlice`, even for empty slices. -For an empty source dimension, do not derive an upper bound. -For a nonempty legal dimension, the inclusive upper bound `bound + (length - 1)` is representable. -When `high >= low`, `bound <= low <= high <= upper` bounds the count by the source length. -Compare before subtraction. Do not calculate the exclusive endpoint or use widened arithmetic as an unrelated rewrite. -Do not reject all negative indices. Negative absolute indices are valid for negative-based arrays. - -Replace every retained-dimension count in these eleven bodies: - -| Getter body | Counts to replace | -|---|---:| -| `GetArraySlice` | 1 | -| `GetArraySlice2D` | 2 | -| `GetArraySlice2DFixed` | 1 | -| `GetArraySlice3D` | 3 | -| `GetArraySlice3DFixedSingle` | 2 | -| `GetArraySlice3DFixedDouble` | 1 | -| `GetArraySlice4D` | 4 | -| `GetArraySlice4DFixedSingle` | 3 | -| `GetArraySlice4DFixedDouble` | 2 | -| `GetArraySlice4DFixedTriple` | 1 | -| `GetStringSlice` | 1 | - -For example, `GetArraySlice` becomes a `(start, len)` helper call followed by `GetArraySub source start len`. -For each multidimensional getter, obtain `(startN, lenN)` for each retained axis. -Feed those counts to the existing allocation helpers and loops. -Keep `GetArraySub`, `GetArray2DSub`, `GetArray3DSub`, and `GetArray4DSub` at lines 799-922 unchanged. -Keep their copy behavior, all fixed-getter source element expressions, dimension reads, and match-based validation order. -Existing allocation clamps can remain. No negative count can reach a getter loop after normalization. -Do not replace a multidimensional empty result with an all-zero shape or a rank-one empty array. - -Do not change setters, reverse-index translation, compiler lowering, signatures, diagnostics, public APIs, or baseline files. -In particular, do not repair missing source offsets inside the 3D/4D fixed getter match arms. -Avoid new generic abstractions, an extra shape framework, duplicate setup, and explanatory comment blocks. - -### 4. Rebuild and verify actual consumers GREEN - -Rebuild the Debug composite after the implementation change. -Recompile the test consumer before rerunning the exact RED cases. -Do not weaken expected shapes or convert expected-empty assertions into exception expectations. -Do not use `--no-build` against stale consumers. - -Public inline arithmetic can be embedded in consumers. -A Core DLL replacement does not necessarily repair previously compiled code. -Record the referenced and loaded Core path, target framework, and file hash or module identity for each validation leg. -A plain `dotnet fsi` session can load SDK Core and is not proof of the local fix. - -Run the regression selection against freshly compiled consumers of `netstandard2.0`, `netstandard2.1`, and the default shipped-net Core. -Use the same compact tests. Do not create a permanent target/platform matrix harness. -One local route is to rebuild the unit-test consumer with its existing Core-reference property overridden: - -```powershell -$env:BUILDING_USING_DOTNET = 'true' -dotnet build tests\FSharp.Core.UnitTests\FSharp.Core.UnitTests.fsproj -c Debug -t:Rebuild -p:BuildProjectReferences=false -p:FSharpCoreShippedNetTargetFramework=netstandard2.0 -dotnet exec artifacts\bin\FSharp.Core.UnitTests\Debug\net11.0\FSharp.Core.UnitTests.dll --filter-method "*GetterSlicingOverflow*" -``` - -Build all normal dependencies and Core targets through the composite before using `BuildProjectReferences=false`. -This command route is based on project inspection, not an executed planning-time build. Validate its reference resolution before trusting results. -Repeat the consumer rebuild with `netstandard2.1`, then with the actual shipped-net value. -This property selects the test project's `ProjectReference` target. Do not change its `.fsproj`. -Verify the compiler's resolved `/reference:` and loaded Core identity for each run. -Do not assume that an output-directory DLL copy changed the compilation input. -If the property route does not resolve the intended reference, use an ignored temporary consumer with an explicit local DLL reference. -Disable its implicit FSharp.Core package and recompile it separately for each target. -Keep this fallback outside the committed source and reuse the same regression inputs and shape assertions. -Capture the same target distinctions during RED as well as GREEN. - -Exercise both ordinary F# slice syntax and emitted callable intrinsic bodies where needed. -An F# call to an inline intrinsic can inline too, so it is not automatically a callable-body test. -For body execution, use a narrowly scoped reflection invocation or a tiny temporary C# consumer. -The existing reflection pattern in `Array2Module.fs`, `RequiresDynamicCodeIsOnBasedApisOnly`, starts from `typeof.Assembly`. -Resolve only the required public getter wrappers and specialize them to `int` where needed. -Assert the returned shape. Do not build a name-discovery framework or treat `TargetInvocationException` as the expected result. -Verify the actual CLR type/method names rather than assuming F# source names map unchanged. -Make representative body coverage part of the compact regression suite, not only an unrecorded scratch experiment. - -Run broader slicing controls after the focused GREEN selection: - -```powershell -dotnet exec artifacts\bin\FSharp.Core.UnitTests\Debug\net11.0\FSharp.Core.UnitTests.dll --filter-method "*Slicing*" --filter-method "*slice*" --filter-method "*Fixed*" --filter-method "*OptimizedRanges*" -``` - -This includes ordinary setters and reverse slices as controls, not as implementation scope. -Ensure the case-sensitive filters include the lowercase-named empty-slice identity test and new control names. - -Finally, build the Release composite and run the full Core suite with the default shipped Core reference: - -```powershell -.\build.cmd -c Release -noVisualStudio -dotnet exec artifacts\bin\FSharp.Core.UnitTests\Release\net11.0\FSharp.Core.UnitTests.dll -``` - -Run the focused regression selection in Release against both netstandard Core targets as well, using freshly rebuilt consumers. -Do this after the full default-reference suite, or restore the default reference before the full suite. -`SurfaceArea.fs` uses different baselines for netstandard2.0 and the CoreCLR/default surface. -Do not run the default full-suite surface check against a substituted netstandard2.0 Core and then update its baseline. -Keep `TEST_UPDATE_BSL` unset. Existing Core surface checks must pass without baseline changes. -Record all failures explicitly and resolve regressions before marking this sprint done. - -### 5. Format, review, document, and commit - -Format only the changed F# files, not the repository: - -```powershell -dotnet fantomas src\FSharp.Core\prim-types.fs tests\FSharp.Core.UnitTests\FSharp.Core\OperatorsModule1.fs -``` - -If Fantomas is missing, restore the repository tool manifest after that missing-tool failure. -Inspect the diff and retain only formatting associated with the changes. Do not keep unrelated whole-file formatting churn. -Rebuild and rerun affected selections after final edits. - -Invoke the `reviewing-compiler-prs` skill and the `expert-reviewer` agent on the final local diff. -Focus review on Core stability, inline/binary compatibility, API surface, arithmetic bounds, retained shapes, and test completeness. -Supply the actual RED/GREEN evidence and the explicit exclusions. -Resolve actionable findings, remove duplicate setup and unnecessary helpers, and rerun affected validation. -The review is local. Do not post findings to GitHub. - -Invoke `release-notes` after the fix is validated. -Read `gh variable get VNEXT --repo dotnet/fsharp`; its planning-time value was `11.0.100`. -Use the skill's insertion helper for the current `.FSharp.Core` file and its `Fixed` section. -Add one short entry such as: - -```markdown -* Fix array and string slices with extreme reversed bounds to return correctly shaped empty results. ([Issue #20530](https://github.com/dotnet/fsharp/issues/20530)) -``` - -There is no PR in this commit-only workflow. Do not fabricate a PR link or open a PR to obtain one. -Do not claim that replacing Core repairs old inline consumers. Explain recompilation in the commit's validation summary. - -Remove temporary consumer projects and copied binaries after retaining their commands and results. -Keep evidence outside cleaned build outputs so it survives another execution window. -Inspect `git diff --check`, the complete product diff, and `git status`. -Stage only the intended Core, test, and release-note paths. Never use a broad add that captures runner artifacts. -Commit with a descriptive message that includes a concise RED/GREEN, Core-target, and review summary. -Include the trailers required by your execution session's instructions. -Do not amend another agent's commit. Do not push. - -## Definition of Done - -- Correct expected-empty tests were added and executed before changing `prim-types.fs`, with durable RED evidence. -- The original safe string symptom and both safe extreme 1D array ranges return empty without exception. -- Full 2D, 3D, and 4D tests rotate the reversed range through every retained axis and assert rank, all lengths, and all zero lower bounds. -- Tests reach all six fixed getter implementations without duplicating every public wrapper. -- Fixed `1..Int32.MinValue` cases return correctly shaped empty results in 2D, 3D, and 4D without entering element access. -- Positive-based, negative-based, valid negative absolute-index, legal `Int32.MaxValue` endpoint, and empty `Int32.MinValue`-based cases pass. -- Exactly one implementation-only getter normalizer covers all eleven getter bodies and twenty-one dimension counts. -- The normalizer preserves the old low exactly, handles zero length before upper-bound arithmetic, and compares endpoints before subtraction. -- The existing `ComputeSlice`, setters, fixed source-offset expressions, compiler code, public signatures, and baseline files are unchanged. -- The same regression assertions are GREEN after recompiling consumers, without weakened expected values or shapes. -- Fresh consumer execution is recorded for `netstandard2.0`, `netstandard2.1`, and the actual shipped-net Core target, with resolved and loaded assembly identities. -- Ordinary F# slicing and representative callable intrinsic bodies are both exercised, with their RED/GREEN outcomes distinguished. -- Null exceptions, fixed-index validation timing, list controls, clipping, omitted bounds, empty/copy identity, ordinary setters, and reverse slices retain their existing behavior. -- The repository composite Release build, focused sibling selection, full Core unit-test suite, and unchanged surface-area checks pass locally. -- Only changed F# files were formatted, and the final diff contains no unrelated formatting or generated-file changes. -- The requested expert review completed, actionable findings were resolved, and affected validation was rerun. -- One concise Core release note links the issue without a fabricated PR or a promise about already compiled inline consumers. -- Intended changes are committed with durable validation details and required trailers, with no uncommitted task edits or temporary consumer files remaining. -- Nothing was pushed, no PR was created, and no GitHub comment or review was posted.