From 62ee3402b0065bec2f388b5cababf28e7cd80c68 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Wed, 12 Aug 2026 10:56:03 +0200 Subject: [PATCH 1/4] Fix recursive inline SRTP resolution truncated by one currying level 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 --- azure-pipelines-PR.yml | 11 ++-- .../.FSharp.Compiler.Service/11.0.100.md | 1 + src/Compiler/Checking/ConstraintSolver.fs | 15 ++++- .../IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs | 63 +++++++++++++++++++ 4 files changed, 83 insertions(+), 7 deletions(-) diff --git a/azure-pipelines-PR.yml b/azure-pipelines-PR.yml index 65f45277382..92af793876e 100644 --- a/azure-pipelines-PR.yml +++ b/azure-pipelines-PR.yml @@ -760,24 +760,23 @@ stages: useVmImage: $(LinuxMachineQueueName) usePool: $(DncEngPublicBuildPool) - repo: fsprojects/FSharpPlus - commit: 2648efe + commit: f42f81885111c652b08218e0880c264447ae56e4 buildScript: dotnet build tests/FSharpPlus.Tests/FSharpPlus.Tests.fsproj -c Release -bl displayName: FsharpPlus_NET10_Build_Lib_Tests - # remove this before merging - repo: fsprojects/FSharpPlus - commit: 2648efe + commit: f42f81885111c652b08218e0880c264447ae56e4 buildScript: dotnet msbuild build.proj -t:Build;Test -bl displayName: FsharpPlus_NET10_Test_Debug - repo: fsprojects/FSharpPlus - commit: 2648efe + commit: f42f81885111c652b08218e0880c264447ae56e4 buildScript: dotnet msbuild build.proj -t:Build;Test -p:Configuration=Release -bl displayName: FsharpPlus_NET10_Test_Release - repo: fsprojects/FSharpPlus - commit: 2648efe + commit: f42f81885111c652b08218e0880c264447ae56e4 buildScript: dotnet msbuild build.proj -t:Build;AllDocs -bl displayName: FsharpPlus_NET10_Docs - repo: fsprojects/FSharpPlus - commit: 2648efe + commit: f42f81885111c652b08218e0880c264447ae56e4 buildScript: build.sh displayName: FsharpPlus_Net10_Linux useVmImage: $(LinuxMachineQueueName) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index 1058b723552..932940c0500 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,5 +1,6 @@ ### Fixed +* Fix recursive inline SRTP resolution being truncated by one currying level (e.g. FSharpPlus `memoizeN`), a regression from the function-domain unification order change in [PR #15181](https://github.com/dotnet/fsharp/pull/15181); the contravariant domain now keeps the inference variable that still carries the pending member constraint. * Fix incorrect `StructLayout(Size = 1)` emission for data-less struct unions where the compiler-generated tag field makes the actual runtime size larger. ([PR #19759](https://github.com/dotnet/fsharp/pull/19759)) * Fix FS0750 "This construct may only be used within computation expressions" incorrectly raised for `let!`/`use!`/`do!` appearing in the right-hand side of a plain `let` binding inside a computation expression. The right-hand side is now desugared as a nested computation of the same builder whose result is bound with `let!`, keeping its bindings correctly scoped. ([Issue #19457](https://github.com/dotnet/fsharp/issues/19457), [PR #19868](https://github.com/dotnet/fsharp/pull/19868)) * Stop leaking a `System.Diagnostics.Metrics.MeterListener` per `Cache` in DEBUG builds. Each cache created a `CacheMetrics.CacheMetricsListener` (which starts a `MeterListener` registered in the process-global metrics registry) and never disposed it, so listeners accumulated for the lifetime of the process. Because every cache hit/miss/add published to all registered listeners, the per-operation cost grew linearly with the number of leaked listeners, so repeated checks (and Debug FCS test runs) slowed down over time. The per-cache `CacheMetricsListener` and the per-instance `cacheId` tag are removed; `DebugDisplay` and tests now read the existing name-aggregated stats populated by the single `ListenToAll` listener, so no per-cache listener is created and no per-operation cost is added. ([PR #19995](https://github.com/dotnet/fsharp/pull/19995)) diff --git a/src/Compiler/Checking/ConstraintSolver.fs b/src/Compiler/Checking/ConstraintSolver.fs index b4e8d5380c1..9b4346776c1 100644 --- a/src/Compiler/Checking/ConstraintSolver.fs +++ b/src/Compiler/Checking/ConstraintSolver.fs @@ -1502,7 +1502,20 @@ and SolveFunTypeEqn csenv ndeep m2 trace cxsln domainTy1 domainTy2 rangeTy1 rang trackErrors { let g = csenv.g let domainTy2 = reqTyForArgumentNullnessInference g domainTy1 domainTy2 - do! SolveTypeEqualsTypeKeepAbbrevsWithCxsln csenv ndeep m2 trace cxsln domainTy2 domainTy1 + // Keep an inference variable that still carries an unsolved SRTP constraint as the + // unification representative: if the required domain absorbs it, the pending recursive + // trait resolution is merged away and recursive SRTP specialization is truncated by one + // currying level. This restores the forward domain order that nullness PR #15181 reversed, + // but only for that case; skipped under MatchingOnly, where only the left type variable may + // be solved (see SolveTypeEqualsType). + let inline isUnsolvedTraitTypar ty = + match tryDestTyparTy g ty with + | ValueSome tp -> tp |> HasConstraint (function TyparConstraint.MayResolveMember(traitInfo, _) -> traitInfo.Solution.IsNone | _ -> false) + | _ -> false + if not csenv.MatchingOnly && isUnsolvedTraitTypar domainTy2 && not (isUnsolvedTraitTypar domainTy1) then + do! SolveTypeEqualsTypeKeepAbbrevsWithCxsln csenv ndeep m2 trace cxsln domainTy1 domainTy2 + else + do! SolveTypeEqualsTypeKeepAbbrevsWithCxsln csenv ndeep m2 trace cxsln domainTy2 domainTy1 return! SolveTypeEqualsTypeKeepAbbrevsWithCxsln csenv ndeep m2 trace cxsln rangeTy1 rangeTy2 } diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs index fb6c648ca49..3de8d80d947 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs @@ -1970,6 +1970,69 @@ let resultInt: int = call 42 if resultFloat <> 0.0 then failwith $"Expected 0.0 but got {resultFloat}" if resultDecimal <> 0M then failwith $"Expected 0M but got {resultDecimal}" if resultInt <> 0 then failwith $"Expected 0 but got {resultInt}" +""" + |> asExe + |> compileAndRun + |> shouldSucceed + + // Recursive inline SRTP resolution must not be truncated by one currying level (regression from + // the domain-order reversal in nullness PR #15181). + [] + let ``Recursive inline SRTP memoization specializes at every currying depth`` () = + FSharp """ +module Test +open System.Collections.Concurrent + +type Default1 = class end + +[] +type MemoizationKeyWrapper<'a> = MemoizationKeyWrapper of 'a + +type MemoizeN = + inherit Default1 + static member getOrAdd (cd: ConcurrentDictionary,'b>) (f: 'a -> 'b) k = + cd.GetOrAdd (MemoizationKeyWrapper k, (fun (MemoizationKeyWrapper x) -> x) >> f) + +let inline memoizeN (f: ^F) : ^F = + let inline call_2 (a: ^MemoizeN, b: ^b) = ((^MemoizeN or ^b) : (static member MemoizeN : ^MemoizeN * 'b -> _ ) (a, b)) + call_2 (Unchecked.defaultof, Unchecked.defaultof< ^F >) f + +type MemoizeN with + static member MemoizeN (_: Default1, _: 'a -> 'b) = MemoizeN.getOrAdd (ConcurrentDictionary ()) + static member inline MemoizeN (_: MemoizeN, _:'t -> 'a -> 'b) = MemoizeN.getOrAdd (ConcurrentDictionary ()) << (<<) memoizeN + +let effs = ResizeArray () +let sum3 a (b:int) c = effs.Add "sum3"; a + b + c +let msum3 = memoizeN sum3 +msum3 1 2 3 |> ignore +msum3 1 2 3 |> ignore +if effs.Count <> 1 then failwith $"depth-3 memoization ran the function {effs.Count} times, expected 1" + +let effs2 = ResizeArray () +let sum2 (a:int) (b:int) = effs2.Add "sum2"; a + b +let msum2 = memoizeN sum2 +msum2 1 1 |> ignore +msum2 1 1 |> ignore +if effs2.Count <> 1 then failwith $"depth-2 memoization ran the function {effs2.Count} times, expected 1" +""" + |> asExe + |> compileAndRun + |> shouldSucceed + + // Guards the `not csenv.MatchingOnly` gate of the SolveFunTypeEqn SRTP fix (mirrors the same + // guard in SolveTypeEqualsType): an SRTP-constrained argument must not disturb overload + // candidate selection, else the lambda's type is left uninferred (FS0072). + [] + let ``SRTP argument does not disturb overload resolution during MatchingOnly`` () = + FSharp """ +module Test +let inline dbl x = x + x +type K = + static member M(g: int -> int, f: string -> int) = f "a" + static member M(g: System.DateTime -> System.DateTime, f: System.DateTime -> int) = 0 + +let r = K.M(dbl, fun v -> v.Length) +if r <> 1 then failwith $"Expected 1 but got {r}" """ |> asExe |> compileAndRun From bd9fd6637fde171170df02058632b53a0699ea6e Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Wed, 12 Aug 2026 13:38:02 +0200 Subject: [PATCH 2/4] Add PR link to memoizeN SRTP fix release note Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4cfb1c86-0d7e-45e6-9529-3c5f301c0912 --- docs/release-notes/.FSharp.Compiler.Service/11.0.100.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index 932940c0500..076666dea97 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,6 +1,6 @@ ### Fixed -* Fix recursive inline SRTP resolution being truncated by one currying level (e.g. FSharpPlus `memoizeN`), a regression from the function-domain unification order change in [PR #15181](https://github.com/dotnet/fsharp/pull/15181); the contravariant domain now keeps the inference variable that still carries the pending member constraint. +* Fix recursive inline SRTP resolution being truncated by one currying level (e.g. FSharpPlus `memoizeN`), a regression from the function-domain unification order change in [PR #15181](https://github.com/dotnet/fsharp/pull/15181); the contravariant domain now keeps the inference variable that still carries the pending member constraint. ([PR #20247](https://github.com/dotnet/fsharp/pull/20247)) * Fix incorrect `StructLayout(Size = 1)` emission for data-less struct unions where the compiler-generated tag field makes the actual runtime size larger. ([PR #19759](https://github.com/dotnet/fsharp/pull/19759)) * Fix FS0750 "This construct may only be used within computation expressions" incorrectly raised for `let!`/`use!`/`do!` appearing in the right-hand side of a plain `let` binding inside a computation expression. The right-hand side is now desugared as a nested computation of the same builder whose result is bound with `let!`, keeping its bindings correctly scoped. ([Issue #19457](https://github.com/dotnet/fsharp/issues/19457), [PR #19868](https://github.com/dotnet/fsharp/pull/19868)) * Stop leaking a `System.Diagnostics.Metrics.MeterListener` per `Cache` in DEBUG builds. Each cache created a `CacheMetrics.CacheMetricsListener` (which starts a `MeterListener` registered in the process-global metrics registry) and never disposed it, so listeners accumulated for the lifetime of the process. Because every cache hit/miss/add published to all registered listeners, the per-operation cost grew linearly with the number of leaked listeners, so repeated checks (and Debug FCS test runs) slowed down over time. The per-cache `CacheMetricsListener` and the per-instance `cacheId` tag are removed; `DebugDisplay` and tests now read the existing name-aggregated stats populated by the single `ListenToAll` listener, so no per-cache listener is created and no per-operation cost is added. ([PR #19995](https://github.com/dotnet/fsharp/pull/19995)) From f1a99e98e35ca15522d7bc0b80e64f004af1c0c1 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Thu, 13 Aug 2026 12:00:56 +0200 Subject: [PATCH 3/4] Compact SRTP fix comment and memoization test 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 --- src/Compiler/Checking/ConstraintSolver.fs | 10 ++++------ .../IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs | 17 ++++------------- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/Compiler/Checking/ConstraintSolver.fs b/src/Compiler/Checking/ConstraintSolver.fs index 9b4346776c1..4e042feb775 100644 --- a/src/Compiler/Checking/ConstraintSolver.fs +++ b/src/Compiler/Checking/ConstraintSolver.fs @@ -1502,12 +1502,10 @@ and SolveFunTypeEqn csenv ndeep m2 trace cxsln domainTy1 domainTy2 rangeTy1 rang trackErrors { let g = csenv.g let domainTy2 = reqTyForArgumentNullnessInference g domainTy1 domainTy2 - // Keep an inference variable that still carries an unsolved SRTP constraint as the - // unification representative: if the required domain absorbs it, the pending recursive - // trait resolution is merged away and recursive SRTP specialization is truncated by one - // currying level. This restores the forward domain order that nullness PR #15181 reversed, - // but only for that case; skipped under MatchingOnly, where only the left type variable may - // be solved (see SolveTypeEqualsType). + // Keep a typar that still carries an unsolved SRTP constraint as the unification + // representative; otherwise its pending recursive trait resolution is merged away and + // specialization is truncated by one currying level (regressed by #15181). Under + // MatchingOnly only the left type variable may be solved, so keep the original order. let inline isUnsolvedTraitTypar ty = match tryDestTyparTy g ty with | ValueSome tp -> tp |> HasConstraint (function TyparConstraint.MayResolveMember(traitInfo, _) -> traitInfo.Solution.IsNone | _ -> false) diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs index 3de8d80d947..49ca1bc72f7 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs @@ -2001,27 +2001,18 @@ type MemoizeN with static member MemoizeN (_: Default1, _: 'a -> 'b) = MemoizeN.getOrAdd (ConcurrentDictionary ()) static member inline MemoizeN (_: MemoizeN, _:'t -> 'a -> 'b) = MemoizeN.getOrAdd (ConcurrentDictionary ()) << (<<) memoizeN -let effs = ResizeArray () -let sum3 a (b:int) c = effs.Add "sum3"; a + b + c +let mutable calls = 0 +let sum3 a (b:int) c = calls <- calls + 1; a + b + c let msum3 = memoizeN sum3 msum3 1 2 3 |> ignore msum3 1 2 3 |> ignore -if effs.Count <> 1 then failwith $"depth-3 memoization ran the function {effs.Count} times, expected 1" - -let effs2 = ResizeArray () -let sum2 (a:int) (b:int) = effs2.Add "sum2"; a + b -let msum2 = memoizeN sum2 -msum2 1 1 |> ignore -msum2 1 1 |> ignore -if effs2.Count <> 1 then failwith $"depth-2 memoization ran the function {effs2.Count} times, expected 1" +if calls <> 1 then failwith $"depth-3 memoization ran {calls} times, expected 1" """ |> asExe |> compileAndRun |> shouldSucceed - // Guards the `not csenv.MatchingOnly` gate of the SolveFunTypeEqn SRTP fix (mirrors the same - // guard in SolveTypeEqualsType): an SRTP-constrained argument must not disturb overload - // candidate selection, else the lambda's type is left uninferred (FS0072). + // Without the `not csenv.MatchingOnly` gate, the lambda's type is left uninferred (FS0072). [] let ``SRTP argument does not disturb overload resolution during MatchingOnly`` () = FSharp """ From 49a903277f732d673b2b9f95d0119ab06a498f89 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Thu, 13 Aug 2026 13:22:42 +0200 Subject: [PATCH 4/4] Add nullness regression test for SRTP memoization under checknulls 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 --- .../IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs index 49ca1bc72f7..57e5cc39fe7 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs @@ -2028,3 +2028,42 @@ if r <> 1 then failwith $"Expected 1 but got {r}" |> asExe |> compileAndRun |> shouldSucceed + + // The domain-order fix keeps the SRTP typar as the unification representative, so a nullable + // argument annotation survives the recursive specialization and nullness inference stays intact + // under --checknulls+ (no lost or spurious warning, no internal error). + [] + let ``Recursive inline SRTP memoization preserves nullable argument inference under checknulls`` () = + FSharp """ +module Test +open System.Collections.Concurrent + +type Default1 = class end + +[] +type MemoizationKeyWrapper<'a> = MemoizationKeyWrapper of 'a + +type MemoizeN = + inherit Default1 + static member getOrAdd (cd: ConcurrentDictionary,'b>) (f: 'a -> 'b) k = + cd.GetOrAdd (MemoizationKeyWrapper k, (fun (MemoizationKeyWrapper x) -> x) >> f) + +let inline memoizeN (f: ^F) : ^F = + let inline call_2 (a: ^MemoizeN, b: ^b) = ((^MemoizeN or ^b) : (static member MemoizeN : ^MemoizeN * 'b -> _ ) (a, b)) + call_2 (Unchecked.defaultof, Unchecked.defaultof< ^F >) f + +type MemoizeN with + static member MemoizeN (_: Default1, _: 'a -> 'b) = MemoizeN.getOrAdd (ConcurrentDictionary ()) + static member inline MemoizeN (_: MemoizeN, _:'t -> 'a -> 'b) = MemoizeN.getOrAdd (ConcurrentDictionary ()) << (<<) memoizeN + +let lookup (prefix: int) (index: int) (s: string | null) = s.Length + prefix + index +let mlookup = memoizeN lookup +mlookup 1 2 "abc" |> ignore +""" + |> withLangVersion "preview" + |> withCheckNulls + |> withWarnOn 3261 + |> compile + |> withDiagnostics [ + Warning 3261, Line 23, Col 60, Line 23, Col 61, "Nullness warning: Possible dereference of a null value when accessing member 'Length' on the nullable value 's' of type 'string | null'." + ]