Skip to content

Add nullness regression test for recursive inline SRTP memoization - #20247

Open
T-Gro wants to merge 6 commits into
mainfrom
t-gro-fix-memoizen-srtp-regression
Open

Add nullness regression test for recursive inline SRTP memoization#20247
T-Gro wants to merge 6 commits into
mainfrom
t-gro-fix-memoizen-srtp-regression

Conversation

@T-Gro

@T-Gro T-Gro commented Aug 12, 2026

Copy link
Copy Markdown
Member

The recursive inline SRTP resolution fix (FSharpPlus memoizeN) landed in #19785. This adds the one piece that PR didn't carry: a --checknulls+ regression test locking that the domain-order fix stays nullness-neutral — a nullable argument annotation still flows through the recursive specialization and produces the same precise FS3261 dereference warning, with no lost or spurious warning and no internal error.

The function-domain unification order change in PR #15181 (nullness) caused
recursive inline SRTP resolution to be truncated by one currying level: an
inference variable still carrying an unsolved member constraint could be
absorbed by the required domain, merging away the pending recursive trait
resolution. This regressed patterns such as FSharpPlus `memoizeN`.

SolveFunTypeEqn now keeps the SRTP-constrained inference variable as the
unification representative for that specific case, and only when not
MatchingOnly (mirroring SolveTypeEqualsType).

Adds two ComponentTests (the memoizeN repro across currying depths, and a
MatchingOnly overload-resolution guard) and repoints the FSharpPlus
regression matrix at a minimal global.json-bump branch that exercises the
fix end-to-end.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4cfb1c86-0d7e-45e6-9529-3c5f301c0912
@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

✅ No release notes required

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4cfb1c86-0d7e-45e6-9529-3c5f301c0912
@T-Gro
T-Gro marked this pull request as ready for review August 12, 2026 18:47
@T-Gro
T-Gro requested a review from a team as a code owner August 12, 2026 18:47
@github-actions github-actions Bot added the AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed label Aug 12, 2026
bartelink added a commit to bartelink/fsharp that referenced this pull request Aug 12, 2026
bartelink added a commit to bartelink/fsharp that referenced this pull request Aug 12, 2026
T-Gro and others added 2 commits August 13, 2026 12:00
Trim the SolveFunTypeEqn comment to the why + bug link + MatchingOnly
invariant, drop the depth-2 memoization control block (depth-3 is the
minimal reproducing case), and count invocations with a mutable int
instead of a ResizeArray of unused labels.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4cfb1c86-0d7e-45e6-9529-3c5f301c0912
Locks that the domain-order fix in SolveFunTypeEqn keeps recursive inline
SRTP memoization nullness-neutral under --checknulls+: a nullable argument
annotation still flows through the recursive specialization and produces the
same precise FS3261 deref warning (no lost/spurious warning, no internal error).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4cfb1c86-0d7e-45e6-9529-3c5f301c0912
@T-Gro
T-Gro requested a review from abonie August 13, 2026 13:35
T-Gro added 2 commits August 13, 2026 15:39
…tp-regression

# Conflicts:
#	azure-pipelines-PR.yml
…tp-regression

# Conflicts:
#	src/Compiler/Checking/ConstraintSolver.fs
#	tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs
@T-Gro T-Gro changed the title Fix recursive inline SRTP resolution truncated by one currying level Add nullness regression test for recursive inline SRTP memoization Aug 13, 2026
@github-project-automation github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants