Fix struct/closure byref capture and let-rec initialization order

Fix 3 codegen bugs in closure generation and initialization:
- #19068: Object expressions in struct types generate invalid IL with byref fields
- #17692: Duplicate parameter names in closure constructors
- #12384: let-rec mutual recursion initialization order for mixed lambda/value bindings

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: T-Gro

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

@@ -21,6 +21,9 @@
 * Fix FS3356 false positive for instance extension members with same name on different types, introduced by [#18821](https://github.com/dotnet/fsharp/pull/18821). ([PR #19260](https://github.com/dotnet/fsharp/pull/19260))
 * Fix graph-based type checking incorrectly resolving dependencies when the same module name is defined across multiple files in the same namespace. ([PR #19280](https://github.com/dotnet/fsharp/pull/19280))
 * F# Scripts: Fix default reference paths resolving when an SDK directory is specified. ([PR #19270](https://github.com/dotnet/fsharp/pull/19270))
+* Fix object expressions in struct types no longer generate invalid IL with byref fields. (Issue [#19068](https://github.com/dotnet/fsharp/issues/19068), [PR #19339](https://github.com/dotnet/fsharp/pull/19339))
+* Avoid duplicate parameter names in closure constructors. (Issue [#17692](https://github.com/dotnet/fsharp/issues/17692), [PR #19339](https://github.com/dotnet/fsharp/pull/19339))
+* Improve let-rec codegen: reorder bindings to allocate lambda closures before non-lambda values that reference them. ([PR #19339](https://github.com/dotnet/fsharp/pull/19339))
 
 ### Added
 * FSharpType: add ImportILType ([PR #19300](https://github.com/dotnet/fsharp/pull/19300))

src/Compiler/CodeGen/EraseClosures.fs

@@ -392,6 +392,21 @@ let mkILFreeVarForParam (p: ILParameter) =
 
 let mkILLocalForFreeVar (p: IlxClosureFreeVar) = mkILLocal p.fvType None
 
+// Note: This is similar to ChooseFreeVarNames in IlxGen.fs but operates on
+// IlxClosureFreeVar[] instead of string lists. Kept separate to avoid cross-file dependency.
+let mkUniqueFreeVarName (baseName: string) (existingFields: IlxClosureFreeVar[]) =
+    let existingNames = existingFields |> Array.map (fun fv -> fv.fvName) |> Set.ofArray
+
+    let rec findUnique n =
+        let candidate = if n = 0 then baseName else baseName + string n
+
+        if Set.contains candidate existingNames then
+            findUnique (n + 1)
+        else
+            candidate
+
+    findUnique 0
+
 let mkILCloFldSpecs _cenv flds =
     flds |> Array.map (fun fv -> (fv.fvName, fv.fvType)) |> Array.toList
 
@@ -490,7 +505,8 @@ let rec convIlxClosureDef cenv encl (td: ILTypeDef) clo =
                 let laterGenericParams = td.GenericParams @ addedGenParams
 
                 let selfFreeVar =
-                    mkILFreeVar (CompilerGeneratedName("self" + string nowFields.Length), true, nowCloSpec.ILType)
+                    let baseName = CompilerGeneratedName("self" + string nowFields.Length)
+                    mkILFreeVar (mkUniqueFreeVarName baseName nowFields, true, nowCloSpec.ILType)
 
                 let laterFields = Array.append nowFields [| selfFreeVar |]
                 let laterCloRef = IlxClosureRef(laterTypeRef, laterStruct, laterFields)
@@ -612,7 +628,8 @@ let rec convIlxClosureDef cenv encl (td: ILTypeDef) clo =
                 let laterGenericParams = td.GenericParams
                 // Number each argument left-to-right, adding one to account for the "this" pointer
                 let selfFreeVar =
-                    mkILFreeVar (CompilerGeneratedName "self", true, nowCloSpec.ILType)
+                    let baseName = CompilerGeneratedName "self"
+                    mkILFreeVar (mkUniqueFreeVarName baseName nowFields, true, nowCloSpec.ILType)
 
                 let argToFreeVarMap =
                     (0, selfFreeVar)

src/Compiler/CodeGen/IlxGen.fs

@@ -6450,7 +6450,7 @@ and GenObjectExpr cenv cgbuf eenvouter objExpr (baseType, baseValOpt, basecall,
     GenWitnessArgsFromWitnessInfos cenv cgbuf eenvouter m cloinfo.cloWitnessInfos
 
     for fv in cloinfo.cloFreeVars do
-        GenGetLocalVal cenv cgbuf eenvouter m fv None
+        GenGetFreeVarForClosure cenv cgbuf eenvouter m fv
 
     CG.EmitInstr
         cgbuf
@@ -6541,7 +6541,7 @@ and GenSequenceExpr
                          if stateVarsSet.Contains fv then
                              GenDefaultValue cenv cgbuf eenv (fv.Type, m)
                          else
-                             GenGetLocalVal cenv cgbuf eenv m fv None
+                             GenGetFreeVarForClosure cenv cgbuf eenv m fv
 
                      CG.EmitInstr
                          cgbuf
@@ -6653,7 +6653,7 @@ and GenSequenceExpr
         if stateVarsSet.Contains fv then
             GenDefaultValue cenv cgbuf eenvouter (fv.Type, m)
         else
-            GenGetLocalVal cenv cgbuf eenvouter m fv None
+            GenGetFreeVarForClosure cenv cgbuf eenvouter m fv
 
     CG.EmitInstr cgbuf (pop ilCloAllFreeVars.Length) (Push [ ilCloRetTyOuter ]) (I_newobj(ilxCloSpec.Constructor, None))
     GenSequel cenv eenvouter.cloc cgbuf sequel
@@ -6886,7 +6886,9 @@ and GenClosureAlloc cenv (cgbuf: CodeGenBuffer) eenv (cloinfo, m) =
         CG.EmitInstr cgbuf (pop 0) (Push [ EraseClosures.mkTyOfLambdas cenv.ilxPubCloEnv cloinfo.ilCloLambdas ]) (mkNormalLdsfld fspec)
     else
         GenWitnessArgsFromWitnessInfos cenv cgbuf eenv m cloinfo.cloWitnessInfos
-        GenGetLocalVals cenv cgbuf eenv m cloinfo.cloFreeVars
+
+        for fv in cloinfo.cloFreeVars do
+            GenGetFreeVarForClosure cenv cgbuf eenv m fv
 
         CG.EmitInstr
             cgbuf
@@ -6901,6 +6903,12 @@ and GenLambda cenv cgbuf eenv isLocalTypeFunc thisVars expr sequel =
 
 and GenTypeOfVal cenv eenv (v: Val) = GenType cenv v.Range eenv.tyenv v.Type
 
+and capturedTypeForFreeVar (g: TcGlobals) (fv: Val) =
+    if isByrefTy g fv.Type then
+        destByrefTy g fv.Type
+    else
+        fv.Type
+
 and GenFreevar cenv m eenvouter tyenvinner (fv: Val) =
     let g = cenv.g
 
@@ -6915,7 +6923,7 @@ and GenFreevar cenv m eenvouter tyenvinner (fv: Val) =
     | Method _
     | Null -> error (InternalError("GenFreevar: compiler error: unexpected unrealized value", fv.Range))
 #endif
-    | _ -> GenType cenv m tyenvinner fv.Type
+    | _ -> GenType cenv m tyenvinner (capturedTypeForFreeVar g fv)
 
 and GetIlxClosureFreeVars cenv m (thisVars: ValRef list) boxity eenv takenNames expr =
     let g = cenv.g
@@ -7276,7 +7284,9 @@ and GenDelegateExpr cenv cgbuf eenvouter expr (TObjExprMethod(slotsig, _attribs,
             IlxClosureSpec.Create(IlxClosureRef(ilDelegeeTypeRef, ilCloLambdas, ilCloAllFreeVars), ctxtGenericArgsForDelegee, false)
 
         GenWitnessArgsFromWitnessInfos cenv cgbuf eenvouter m cloWitnessInfos
-        GenGetLocalVals cenv cgbuf eenvouter m cloFreeVars
+
+        for fv in cloFreeVars do
+            GenGetFreeVarForClosure cenv cgbuf eenvouter m fv
 
         CG.EmitInstr
             cgbuf
@@ -8206,6 +8216,17 @@ and GenLetRecFixup cenv cgbuf eenv (ilxCloSpec: IlxClosureSpec, e, ilField: ILFi
     GenExpr cenv cgbuf eenv e2 Continue
     CG.EmitInstr cgbuf (pop 2) Push0 (mkNormalStfld (mkILFieldSpec (ilField.FieldRef, ilxCloSpec.ILType)))
 
+and isLambdaBinding (TBind(_, expr, _)) =
+    match stripDebugPoints expr with
+    | Expr.Lambda _
+    | Expr.TyLambda _
+    | Expr.Obj _ -> true
+    | _ -> false
+
+and reorderBindingsLambdasFirst binds =
+    let lambdas, nonLambdas = binds |> List.partition isLambdaBinding
+    lambdas @ nonLambdas
+
 /// Generate letrec bindings
 and GenLetRecBindings cenv (cgbuf: CodeGenBuffer) eenv (allBinds: Bindings, m) (dict: Dictionary<Stamp, ILTypeRef> option) =
 
@@ -8299,8 +8320,11 @@ and GenLetRecBindings cenv (cgbuf: CodeGenBuffer) eenv (allBinds: Bindings, m) (
     let recursiveVars =
         Zset.addList (bindsPossiblyRequiringFixup |> List.map (fun v -> v.Var)) (Zset.empty valOrder)
 
+    let reorderedBindsPossiblyRequiringFixup =
+        reorderBindingsLambdasFirst bindsPossiblyRequiringFixup
+
     let _ =
-        (recursiveVars, bindsPossiblyRequiringFixup)
+        (recursiveVars, reorderedBindsPossiblyRequiringFixup)
         ||> List.fold (fun forwardReferenceSet (bind: Binding) ->
             // Compute fixups
             bind.Expr
@@ -8344,6 +8368,8 @@ and GenLetRecBindings cenv (cgbuf: CodeGenBuffer) eenv (allBinds: Bindings, m) (
     let _ =
         (recursiveVars, groupBinds)
         ||> List.fold (fun forwardReferenceSet (binds: Binding list) ->
+            let binds = reorderBindingsLambdasFirst binds
+
             match dict, cenv.g.realsig, binds with
             | _, false, _
             | None, _, _
@@ -8380,8 +8406,11 @@ and GenLetRecBindings cenv (cgbuf: CodeGenBuffer) eenv (allBinds: Bindings, m) (
 
 and GenLetRec cenv cgbuf eenv (binds, body, m) sequel =
     let _, endMark as scopeMarks = StartLocalScope "letrec" cgbuf
-    let eenv = AllocStorageForBinds cenv cgbuf scopeMarks eenv binds
-    GenLetRecBindings cenv cgbuf eenv (binds, m) None
+
+    let reorderedBinds = reorderBindingsLambdasFirst binds
+
+    let eenv = AllocStorageForBinds cenv cgbuf scopeMarks eenv reorderedBinds
+    GenLetRecBindings cenv cgbuf eenv (reorderedBinds, m) None
     GenExpr cenv cgbuf eenv body (EndLocalScope(sequel, endMark))
 
 //-------------------------------------------------------------------------
@@ -9850,8 +9879,14 @@ and GenGetStorageAndSequel (cenv: cenv) cgbuf eenv m (ty, ilTy) storage storeSeq
         CG.EmitInstrs cgbuf (pop 0) (Push [ ilTy ]) [ mkLdarg0; mkNormalLdfld ilField ]
         CommitGetStorageSequel cenv cgbuf eenv m ty localCloInfo storeSequel
 
-and GenGetLocalVals cenv cgbuf eenvouter m fvs =
-    List.iter (fun v -> GenGetLocalVal cenv cgbuf eenvouter m v None) fvs
+/// Load free variables for closure capture, dereferencing byrefs.
+and GenGetFreeVarForClosure cenv cgbuf eenv m (fv: Val) =
+    let g = cenv.g
+    GenGetLocalVal cenv cgbuf eenv m fv None
+
+    if isByrefTy g fv.Type then
+        let ilUnderlyingTy = GenType cenv m eenv.tyenv (capturedTypeForFreeVar g fv)
+        CG.EmitInstr cgbuf (pop 1) (Push [ ilUnderlyingTy ]) (mkNormalLdobj ilUnderlyingTy)
 
 and GenGetLocalVal cenv cgbuf eenv m (vspec: Val) storeSequel =
     GenGetStorageAndSequel cenv cgbuf eenv m (vspec.Type, GenTypeOfVal cenv eenv vspec) (StorageForVal m vspec eenv) storeSequel