Fix InvalidProgramException from Seq.collect over a struct seq materialised with List.ofSeq/Seq.toList/Seq.toArray

docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@@ -1,5 +1,6 @@
 ### Fixed
 
+* Fix Release-only (`--optimize+`) `System.InvalidProgramException` from `Seq.collect` / `yield!` over a value-type (struct) collection implementing `seq<'T>` (e.g. `ImmutableArray<_>`) when materialised with `List.ofSeq` / `Seq.toList` / `Seq.toArray` or a list/array comprehension. The collector lowering now boxes a struct sub-collection to `seq<'T>` before calling `AddMany`/`AddManyAndClose` (matching the coercion the type checker already inserts for `yield!`), and uses `unit` as the try/finally result type instead of the body type (removing a spurious `ldnull` store). ([Issue #20203](https://github.com/dotnet/fsharp/issues/20203))
 * 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))

src/Compiler/Optimize/LowerComputedCollections.fs

@@ -63,6 +63,8 @@ let mkCallCollectorClose tcVal (g: TcGlobals) infoReader m collExpr =
 let LowerComputedListOrArraySeqExpr tcVal g amap m collectorTy overallSeqExpr =
     let infoReader = InfoReader(g, amap)
     let collVal, collExpr = mkMutableCompGenLocal m "@collector" collectorTy
+    // The seq<'T> element type that AddMany/AddManyAndClose expect for their argument.
+    let collectorSeqTy = mkSeqTy g (List.head (argsOfAppTy g collectorTy))
     //let collExpr = mkValAddr m false (mkLocalValRef collVal)
     let rec ConvertSeqExprCode isUninteresting isTailcall expr =
         match expr with
@@ -98,7 +100,9 @@ let LowerComputedListOrArraySeqExpr tcVal g amap m collectorTy overallSeqExpr =
                 let cleanupE = BuildDisposableCleanup tcVal g infoReader m v
                 let exprR = 
                     mkLet spBind m v resource
-                        (mkTryFinally g (bodyExprR, cleanupE, m, tyOfExpr g bodyExpr, DebugPointAtTry.No, DebugPointAtFinally.No))
+                        // The lowered body is a unit-typed collector call, so the try/finally result type is unit,
+                        // not the struct/ref body type (which would make the optimizer store a spurious default value).
+                        (mkTryFinally g (bodyExprR, cleanupE, m, g.unit_ty, DebugPointAtTry.No, DebugPointAtFinally.No))
                 Result.Ok (false, exprR)
             | Result.Error msg -> Result.Error msg
 
@@ -126,7 +130,8 @@ let LowerComputedListOrArraySeqExpr tcVal g amap m collectorTy overallSeqExpr =
                                     (callNonOverloadedILMethod g amap mIn "get_Current" inpEnumTy [enumve]))
                                     bodyExprR, mIn), 
                             cleanupE,
-                            mFor, tyOfExpr g bodyExpr, DebugPointAtTry.No, DebugPointAtFinally.No))
+                            // Lowered body is unit-typed; the try/finally result type must be unit.
+                            mFor, g.unit_ty, DebugPointAtTry.No, DebugPointAtFinally.No))
                     |> addForDebugPoint
                 Result.Ok (false, exprR)
             | Result.Error msg -> Result.Error msg
@@ -136,7 +141,8 @@ let LowerComputedListOrArraySeqExpr tcVal g amap m collectorTy overallSeqExpr =
             match resBody with 
             | Result.Ok (_, bodyExprR) ->
                 let exprR =
-                    mkTryFinally g (bodyExprR, compensation, m, tyOfExpr g bodyExpr, spTry, spFinally)
+                    // Lowered body is unit-typed; the try/finally result type must be unit.
+                    mkTryFinally g (bodyExprR, compensation, m, g.unit_ty, spTry, spFinally)
                 Result.Ok (false, exprR)
             | Result.Error msg -> Result.Error msg
 
@@ -201,6 +207,12 @@ let LowerComputedListOrArraySeqExpr tcVal g amap m collectorTy overallSeqExpr =
                 // printfn "FAILED - not worth compiling an unrecognized Seq.toList at %s " (stringOfRange m)
                 Result.Error ()
             else
+                // A struct sub-collection must be boxed to seq<'T> before being passed to AddMany/AddManyAndClose:
+                // BuildMethodCall does not coerce actuals, and struct->interface is not a free upcast (unlike ref subtypes).
+                let srcTy = tyOfExpr g arbitrarySeqExpr
+                let arbitrarySeqExpr =
+                    if typeEquiv g srcTy collectorSeqTy then arbitrarySeqExpr
+                    else mkCoerceExpr (arbitrarySeqExpr, collectorSeqTy, m, srcTy)
                 // If we're the final in a sequential chain then we can AddMany, Close and return
                 if isTailcall then 
                     let exprR = mkCallCollectorAddManyAndClose tcVal (g: TcGlobals) infoReader m collExpr arbitrarySeqExpr

tests/FSharp.Compiler.ComponentTests/EmittedIL/ComputedCollections/ComputedCollections.fs

@@ -85,3 +85,9 @@ module ComputedCollections =
         compilation
         |> getCompilation
         |> verifyCompilation
+
+    [<Theory; FileInlineData("StructSeqCollectToList.fs", Realsig = BooleanOptions.Both, Optimize = BooleanOptions.True)>]
+    let ``StructSeqCollectToList_fs`` compilation =
+        compilation
+        |> getCompilation
+        |> verifyCompilation

tests/FSharp.Compiler.ComponentTests/EmittedIL/ComputedCollections/ForNInRangeArrays.fs.il.bsl

@@ -523,9 +523,8 @@
     .maxstack  5
     .locals init (valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32> V_0,
              class [runtime]System.Collections.Generic.IEnumerator`1<int32> V_1,
-             class [runtime]System.Collections.Generic.IEnumerable`1<int32> V_2,
-             int32 V_3,
-             class [runtime]System.IDisposable V_4)
+             int32 V_2,
+             class [runtime]System.IDisposable V_3)
     IL_0000:  nop
     IL_0001:  ldc.i4.1
     IL_0002:  ldc.i4.1
@@ -541,17 +540,17 @@
 
       IL_0012:  ldloc.1
       IL_0013:  callvirt   instance !0 class [runtime]System.Collections.Generic.IEnumerator`1<int32>::get_Current()
-      IL_0018:  stloc.3
+      IL_0018:  stloc.2
       IL_0019:  ldarg.0
       IL_001a:  ldnull
       IL_001b:  callvirt   instance !1 class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<class [FSharp.Core]Microsoft.FSharp.Core.Unit,class [FSharp.Core]Microsoft.FSharp.Core.Unit>::Invoke(!0)
       IL_0020:  pop
       IL_0021:  ldloca.s   V_0
-      IL_0023:  ldloc.3
+      IL_0023:  ldloc.2
       IL_0024:  call       instance void valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32>::Add(!0)
       IL_0029:  nop
       IL_002a:  ldloca.s   V_0
-      IL_002c:  ldloc.3
+      IL_002c:  ldloc.2
       IL_002d:  ldc.i4.1
       IL_002e:  add
       IL_002f:  call       instance void valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32>::Add(!0)
@@ -560,29 +559,25 @@
       IL_0036:  callvirt   instance bool [runtime]System.Collections.IEnumerator::MoveNext()
       IL_003b:  brtrue.s   IL_0012
 
-      IL_003d:  ldnull
-      IL_003e:  stloc.2
-      IL_003f:  leave.s    IL_0056
+      IL_003d:  leave.s    IL_0051
 
     }  
     finally
     {
-      IL_0041:  ldloc.1
-      IL_0042:  isinst     [runtime]System.IDisposable
-      IL_0047:  stloc.s    V_4
-      IL_0049:  ldloc.s    V_4
-      IL_004b:  brfalse.s  IL_0055
-
-      IL_004d:  ldloc.s    V_4
-      IL_004f:  callvirt   instance void [runtime]System.IDisposable::Dispose()
-      IL_0054:  endfinally
-      IL_0055:  endfinally
+      IL_003f:  ldloc.1
+      IL_0040:  isinst     [runtime]System.IDisposable
+      IL_0045:  stloc.3
+      IL_0046:  ldloc.3
+      IL_0047:  brfalse.s  IL_0050
+
+      IL_0049:  ldloc.3
+      IL_004a:  callvirt   instance void [runtime]System.IDisposable::Dispose()
+      IL_004f:  endfinally
+      IL_0050:  endfinally
     }  
-    IL_0056:  ldloc.2
-    IL_0057:  pop
-    IL_0058:  ldloca.s   V_0
-    IL_005a:  call       instance !0[] valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32>::Close()
-    IL_005f:  ret
+    IL_0051:  ldloca.s   V_0
+    IL_0053:  call       instance !0[] valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32>::Close()
+    IL_0058:  ret
   } 
 
   .method public static int32[]  f0000() cil managed
@@ -1024,12 +1019,11 @@
     .maxstack  5
     .locals init (valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32> V_0,
              class [runtime]System.Collections.Generic.IEnumerator`1<int32> V_1,
-             class [runtime]System.Collections.Generic.IEnumerable`1<int32> V_2,
-             int32 V_3,
+             int32 V_2,
+             valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32>& V_3,
              valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32>& V_4,
              valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32>& V_5,
-             valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32>& V_6,
-             class [runtime]System.IDisposable V_7)
+             class [runtime]System.IDisposable V_6)
     IL_0000:  nop
     IL_0001:  ldc.i4.1
     IL_0002:  ldc.i4.1
@@ -1041,60 +1035,56 @@
     IL_000f:  stloc.1
     .try
     {
-      IL_0010:  br.s       IL_004c
+      IL_0010:  br.s       IL_004a
 
       IL_0012:  ldloc.1
       IL_0013:  callvirt   instance !0 class [runtime]System.Collections.Generic.IEnumerator`1<int32>::get_Current()
-      IL_0018:  stloc.3
+      IL_0018:  stloc.2
       IL_0019:  ldloca.s   V_0
-      IL_001b:  stloc.s    V_4
-      IL_001d:  ldloc.s    V_4
-      IL_001f:  ldarg.0
-      IL_0020:  ldnull
-      IL_0021:  callvirt   instance !1 class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<class [FSharp.Core]Microsoft.FSharp.Core.Unit,int32>::Invoke(!0)
-      IL_0026:  call       instance void valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32>::Add(!0)
-      IL_002b:  nop
-      IL_002c:  ldloca.s   V_0
-      IL_002e:  stloc.s    V_5
-      IL_0030:  ldloc.s    V_5
-      IL_0032:  ldarg.1
-      IL_0033:  ldnull
-      IL_0034:  callvirt   instance !1 class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<class [FSharp.Core]Microsoft.FSharp.Core.Unit,int32>::Invoke(!0)
-      IL_0039:  call       instance void valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32>::Add(!0)
-      IL_003e:  nop
-      IL_003f:  ldloca.s   V_0
-      IL_0041:  stloc.s    V_6
-      IL_0043:  ldloc.s    V_6
-      IL_0045:  ldloc.3
-      IL_0046:  call       instance void valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32>::Add(!0)
-      IL_004b:  nop
-      IL_004c:  ldloc.1
-      IL_004d:  callvirt   instance bool [runtime]System.Collections.IEnumerator::MoveNext()
-      IL_0052:  brtrue.s   IL_0012
-
-      IL_0054:  ldnull
-      IL_0055:  stloc.2
-      IL_0056:  leave.s    IL_006d
+      IL_001b:  stloc.3
+      IL_001c:  ldloc.3
+      IL_001d:  ldarg.0
+      IL_001e:  ldnull
+      IL_001f:  callvirt   instance !1 class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<class [FSharp.Core]Microsoft.FSharp.Core.Unit,int32>::Invoke(!0)
+      IL_0024:  call       instance void valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32>::Add(!0)
+      IL_0029:  nop
+      IL_002a:  ldloca.s   V_0
+      IL_002c:  stloc.s    V_4
+      IL_002e:  ldloc.s    V_4
+      IL_0030:  ldarg.1
+      IL_0031:  ldnull
+      IL_0032:  callvirt   instance !1 class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<class [FSharp.Core]Microsoft.FSharp.Core.Unit,int32>::Invoke(!0)
+      IL_0037:  call       instance void valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32>::Add(!0)
+      IL_003c:  nop
+      IL_003d:  ldloca.s   V_0
+      IL_003f:  stloc.s    V_5
+      IL_0041:  ldloc.s    V_5
+      IL_0043:  ldloc.2
+      IL_0044:  call       instance void valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32>::Add(!0)
+      IL_0049:  nop
+      IL_004a:  ldloc.1
+      IL_004b:  callvirt   instance bool [runtime]System.Collections.IEnumerator::MoveNext()
+      IL_0050:  brtrue.s   IL_0012
+
+      IL_0052:  leave.s    IL_0069
 
     }  
     finally
     {
-      IL_0058:  ldloc.1
-      IL_0059:  isinst     [runtime]System.IDisposable
-      IL_005e:  stloc.s    V_7
-      IL_0060:  ldloc.s    V_7
-      IL_0062:  brfalse.s  IL_006c
-
-      IL_0064:  ldloc.s    V_7
-      IL_0066:  callvirt   instance void [runtime]System.IDisposable::Dispose()
-      IL_006b:  endfinally
-      IL_006c:  endfinally
+      IL_0054:  ldloc.1
+      IL_0055:  isinst     [runtime]System.IDisposable
+      IL_005a:  stloc.s    V_6
+      IL_005c:  ldloc.s    V_6
+      IL_005e:  brfalse.s  IL_0068
+
+      IL_0060:  ldloc.s    V_6
+      IL_0062:  callvirt   instance void [runtime]System.IDisposable::Dispose()
+      IL_0067:  endfinally
+      IL_0068:  endfinally
     }  
-    IL_006d:  ldloc.2
-    IL_006e:  pop
-    IL_006f:  ldloca.s   V_0
-    IL_0071:  call       instance !0[] valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32>::Close()
-    IL_0076:  ret
+    IL_0069:  ldloca.s   V_0
+    IL_006b:  call       instance !0[] valuetype [FSharp.Core]Microsoft.FSharp.Core.CompilerServices.ArrayCollector`1<int32>::Close()
+    IL_0070:  ret
   } 
 
   .method public static int32[]  f1() cil managed