Fix recursive inline member ordering in optimizer

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

@@ -110,6 +110,7 @@
 * Reference assembly MVIDs are now deterministic across compiler invocations. Previously, `--refout` / `<ProduceReferenceAssembly>true</ProduceReferenceAssembly>` produced a different MVID every build because the implied signature hash used .NET's randomized `String.GetHashCode()`. ([Issue #19751](https://github.com/dotnet/fsharp/issues/19751), [PR #19801](https://github.com/dotnet/fsharp/pull/19801))
 * Parser: recover on unfinished if and binary expressions
 ([PR #19724](https://github.com/dotnet/fsharp/pull/19724))
+* Fix recursive inline-member optimization dependencies so inline consumers in recursive groups are resolved reliably without changing static initialization order. ([Issue #20085](https://github.com/dotnet/fsharp/issues/20085), [PR #20111](https://github.com/dotnet/fsharp/pull/20111))
 * Fix `SynExpr.shouldBeParenthesizedInContext` to report parentheses as required around `SynExpr.Sequential` expressions used as record or anonymous-record field values, so the IDE "remove unnecessary parentheses" analyzer no longer breaks code like `{| A = ((); B = 3) |}`. ([Issue #17826](https://github.com/dotnet/fsharp/issues/17826), [PR #19850](https://github.com/dotnet/fsharp/pull/19850))
 * Fix semantic classification of `IDisposable` and other interface types in type-occurrence positions being incorrectly classified as `DisposableType` instead of `Interface`. ([Issue #16268](https://github.com/dotnet/fsharp/issues/16268), [PR #19809](https://github.com/dotnet/fsharp/pull/19809))
 * Fix missing semantic classification on second and later type qualifiers in nested copy-and-update expressions like `{ p with Person.Info.X = 1; Person.Info.Y = 2 }`. ([Issue #17428](https://github.com/dotnet/fsharp/issues/17428), [PR #19878](https://github.com/dotnet/fsharp/pull/19878))

src/Compiler/Optimize/Optimizer.fs

@@ -633,7 +633,7 @@ let GetInfoForLocalValue cenv env (v: Val) m =
             match env.localExternalVals.TryFind v.Stamp with 
             | Some vval -> vval
             | None -> 
-                if v.ShouldInline then
+                if cenv.optimizing && v.ShouldInline then
                     errorR(Error(FSComp.SR.optValueMarkedInlineButWasNotBoundInTheOptEnv(fullDisplayTextOfValRef (mkLocalValRef v)), m))
                 UnknownValInfo 
 
@@ -3191,11 +3191,11 @@ and TryOptimizeVal cenv env (vOpt: ValRef option, shouldInline, inlineIfLambda,
     | TupleValue _ | UnionCaseValue _ | RecdValue _ when shouldInline ->
         failwith "tuple, union and record values cannot be marked 'inline'"
 
-    | UnknownValue when shouldInline && cenv.settings.alwaysInline ->
+    | UnknownValue when shouldInline && cenv.settings.alwaysInline && cenv.optimizing ->
         warning(Error(FSComp.SR.optValueMarkedInlineHasUnexpectedValue(), m))
         None
 
-    | _ when shouldInline && cenv.settings.alwaysInline ->
+    | _ when shouldInline && cenv.settings.alwaysInline && cenv.optimizing ->
         warning(Error(FSComp.SR.optValueMarkedInlineCouldNotBeInlined(), m))
         None
 
@@ -3241,7 +3241,7 @@ and OptimizeVal cenv env expr (v: ValRef, m) =
            e, AddValEqualityInfo g m v einfo 
 
     | None ->
-       if cenv.settings.alwaysInline then
+       if cenv.optimizing && cenv.settings.alwaysInline then
            if v.ShouldInline then
                 match valInfoForVal.ValExprInfo with
                 | UnknownValue -> error(Error(FSComp.SR.optFailedToInlineValue(v.DisplayName), m))
@@ -4491,7 +4491,20 @@ and OptimizeBinding cenv isRec env (TBind(vref, expr, spBind)) =
         raise (ReportedError (Some exn))
 
 and OptimizeBindings cenv isRec env xs =
-    List.mapFold (OptimizeBinding cenv isRec) env xs
+    if isRec then
+        let xsArray = xs |> List.toArray
+        let order = GetBindingOptimizationOrder cenv false true xs
+
+        let results, env =
+            (env, order)
+            ||> List.mapFold (fun env idx ->
+                let result, env = OptimizeBinding cenv isRec env xsArray[idx]
+                (idx, result), env)
+
+        let resultsByIndex = results |> Map.ofList
+        [ for idx in 0 .. xsArray.Length - 1 -> resultsByIndex[idx] ], env
+    else
+        List.mapFold (OptimizeBinding cenv isRec) env xs
 
 and OptimizeModuleExprWithSig cenv env mty def  = 
         let g = cenv.g
@@ -4581,11 +4594,109 @@ and OptimizeModuleExprWithSig cenv env mty def  =
 and mkValBind (bind: Binding) info =
     (mkLocalValRef bind.Var, info)
 
+and GetBindingOptimizationOrder cenv inlineDependenciesOnly preferLowArity (binds: Binding list) =
+    // Recursive binding groups are published to the optimizer incrementally as each binding is
+    // processed. If a caller is optimized before a later sibling it depends on, inline lookup can
+    // observe an incomplete optimization environment. Compute a dependency-first schedule for the
+    // recursive group, then restore source order after optimization.
+    let bindsArray = binds |> List.toArray
+
+    let bindIndexByStamp =
+        binds
+        |> List.mapi (fun idx bind -> bind.Var.Stamp, idx)
+        |> Map.ofList
+
+    let addDependency depIdxs stamp =
+        match bindIndexByStamp |> Map.tryFind stamp with
+        | Some depIdx when not inlineDependenciesOnly || bindsArray[depIdx].Var.ShouldInline ->
+            Set.add depIdx depIdxs
+        | None -> depIdxs
+        | Some _ -> depIdxs
+
+    let rec addBindingDependencies depIdxs expr =
+        let addVals depIdxs vals =
+            vals
+            |> Seq.fold (fun depIdxs (v: Val) -> addDependency depIdxs v.Stamp) depIdxs
+
+        let rec addTraitSolutionDependencies depIdxs (traitInfo: TraitConstraintInfo) =
+            match traitInfo.Solution with
+            | Some(FSMethSln(_, vref, _, _)) -> addDependency depIdxs vref.Deref.Stamp
+            | Some(ClosedExprSln witnessExpr) -> addBindingDependencies depIdxs witnessExpr
+            | _ -> depIdxs
+
+        let fvs = freeInExpr CollectLocalsNoCaching expr
+
+        let depIdxs =
+            let depIdxs = addVals depIdxs (fvs.FreeLocals |> Zset.elements)
+            addVals depIdxs (fvs.FreeTyvars.FreeTraitSolutions |> Zset.elements)
+
+        let folder =
+            { ExprFolder0 with
+                exprIntercept =
+                    (fun _exprF noInterceptF depIdxs expr ->
+                        let depIdxs =
+                            match expr with
+                            | Expr.Val(vref, _, _) -> addDependency depIdxs vref.Deref.Stamp
+                            // Member-constraint calls can hide the real sibling dependency behind
+                            // a witness expression, so fold over the resolved witness as well.
+                            | Expr.Op(TOp.TraitCall traitInfo, _, args, m) ->
+                                let depIdxs = addTraitSolutionDependencies depIdxs traitInfo
+
+                                match ConstraintSolver.CodegenWitnessExprForTraitConstraint cenv.TcVal cenv.g cenv.amap m traitInfo args with
+                                | OkResult (_, Some witnessExpr) -> addBindingDependencies depIdxs witnessExpr
+                                | _ -> depIdxs
+                            | _ -> depIdxs
+
+                        noInterceptF depIdxs expr) }
+
+        FoldExpr folder depIdxs expr
+
+    let dependencyIndexes =
+        binds
+        |> List.map (fun (TBind(_, expr, _)) ->
+            addBindingDependencies Set.empty expr |> Set.toArray)
+        |> List.toArray
+
+    let ordered = ResizeArray()
+    let visiting = HashSet<int>()
+    let visited = HashSet<int>()
+
+    let rec visit idx =
+        if not (visited.Contains idx) then
+            if not (visiting.Contains idx) then
+                visiting.Add idx |> ignore
+
+                for depIdx in dependencyIndexes[idx] do
+                    if depIdx <> idx then
+                        visit depIdx
+
+                visiting.Remove idx |> ignore
+                visited.Add idx |> ignore
+                ordered.Add idx
+
+    let rootOrder =
+        [ 0 .. binds.Length - 1 ]
+        |> (if preferLowArity then
+                List.sortBy (fun idx ->
+                    let arity =
+                        bindsArray[idx].Var.ValReprInfo
+                        |> Option.map (fun repr -> repr.TotalArgCount)
+                        |> Option.defaultValue 0
+
+                    arity, -idx)
+            else
+                id)
+
+    for idx in rootOrder do
+        visit idx
+
+    ordered |> Seq.toList
+
 and OptimizeModuleContents cenv (env, bindInfosColl) input = 
     match input with 
     | TMDefRec(isRec, opens, tycons, mbinds, m) -> 
         let env = if isRec then BindInternalValsToUnknown cenv (allValsOfModDef input) env else env
-        let mbindInfos, (env, bindInfosColl) = OptimizeModuleBindings cenv (env, bindInfosColl) mbinds
+        let mbindInfos, (env, bindInfosColl) = OptimizeModuleBindings cenv isRec (env, bindInfosColl) mbinds
         let mbinds, minfos = List.unzip mbindInfos
         let binds = minfos |> List.choose (function Choice1Of2 (x, _) -> Some x | _ -> None)
         let binfos = minfos |> List.choose (function Choice1Of2 (_, x) -> Some x | _ -> None)
@@ -4615,8 +4726,35 @@ and OptimizeModuleContents cenv (env, bindInfosColl) input =
         let (defs, info), (env, bindInfosColl) = OptimizeModuleDefs cenv (env, bindInfosColl) defs 
         (TMDefs defs, info), (env, bindInfosColl)
 
-and OptimizeModuleBindings cenv (env, bindInfosColl) xs =
-    List.mapFold (OptimizeModuleBinding cenv) (env, bindInfosColl) xs
+and OptimizeModuleBindings cenv isRec (env, bindInfosColl) xs =
+    let bindingGroup =
+        xs
+        |> List.map (function
+            | ModuleOrNamespaceBinding.Binding bind -> Some bind
+            | _ -> None)
+
+    let binds = bindingGroup |> List.choose id
+
+    if
+        isRec
+        && (bindingGroup |> List.forall Option.isSome)
+        && (binds |> List.exists (fun bind -> bind.Var.ShouldInline))
+    then
+        let xsArray = xs |> List.toArray
+        let preferLowArity = binds |> List.forall (fun bind -> bind.Var.IsMember)
+        let order = GetBindingOptimizationOrder cenv true preferLowArity binds
+
+        let results, (env, bindInfosColl) =
+            ((env, bindInfosColl), order)
+            ||> List.mapFold (fun state idx ->
+                let result, state = OptimizeModuleBinding cenv state xsArray[idx]
+                (idx, result), state)
+
+        let resultsByIndex = results |> Map.ofList
+        // Keep the emitted binding list in source order; only the optimization schedule changes.
+        [ for idx in 0 .. xsArray.Length - 1 -> resultsByIndex[idx] ], (env, bindInfosColl)
+    else
+        List.mapFold (OptimizeModuleBinding cenv) (env, bindInfosColl) xs
 
 and OptimizeModuleBinding cenv (env, bindInfosColl) x = 
     match x with

tests/AheadOfTime/Trimming/check.ps1

@@ -63,12 +63,12 @@ function CheckTrim($root, $tfm, $outputfile, $expected_len, $callerLineNumber) {
 $allErrors = @()
 
 # Check net9.0 trimmed assemblies.
-$allErrors += CheckTrim -root "SelfContained_Trimming_Test" -tfm "net9.0" -outputfile "FSharp.Core.dll" -expected_len 311296 -callerLineNumber 66
+$allErrors += CheckTrim -root "SelfContained_Trimming_Test" -tfm "net9.0" -outputfile "FSharp.Core.dll" -expected_len 310272 -callerLineNumber 66
 
 # Check net9.0 trimmed assemblies with static linked FSharpCore.
 # Statically links FSharp.Compiler.Service; the size is stable now that its codegen is
 # deterministic (#19928/#19929). Update if compiler/trimming output intentionally changes.
-$allErrors += CheckTrim -root "StaticLinkedFSharpCore_Trimming_Test" -tfm "net9.0" -outputfile "StaticLinkedFSharpCore_Trimming_Test.dll" -expected_len 9174016 -callerLineNumber 71
+$allErrors += CheckTrim -root "StaticLinkedFSharpCore_Trimming_Test" -tfm "net9.0" -outputfile "StaticLinkedFSharpCore_Trimming_Test.dll" -expected_len 9172992 -callerLineNumber 71
 
 # Check net9.0 trimmed assemblies with F# metadata resources removed
 $allErrors += CheckTrim -root "FSharpMetadataResource_Trimming_Test" -tfm "net9.0" -outputfile "FSharpMetadataResource_Trimming_Test.dll" -expected_len 7613440 -callerLineNumber 74

tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/Regression_RecursiveInlineMemberDependencies.fs

@@ -0,0 +1,168 @@
+namespace EmittedIL.Inlining
+
+open Xunit
+open FSharp.Test
+open FSharp.Test.Compiler
+
+module Regression_RecursiveInlineMemberDependencies =
+
+   let private assertCompiles source =
+       source
+       |> withOptimize
+       |> compile
+       |> shouldSucceed
+       |> ignore
+
+   [<Fact>]
+   let ``Inline members that depend on sibling member access compile`` () =
+       FSharp """
+module MemberAccessDependencyRepro
+
+type ValidationBuilder() =
+   member inline _.Return(value: int) : int = value
+
+   member inline this.Bind(value: int, binder: int -> int) : int =
+       let result = this.Source value
+       binder result
+
+   member inline this.Source(value: int) : int = value
+
+let inline run (builder: ValidationBuilder) =
+   builder.Bind(1, fun x -> x + 1)
+"""
+       |> assertCompiles
+
+   [<Fact>]
+   let ``Trait-witness inline overload consumers compile`` () =
+       FSharp """
+module TraitWitnessOverloadRepro
+
+open System.Runtime.InteropServices
+
+type Default1 = class end
+
+type Intersperse =
+   inherit Default1
+
+   static member inline Intersperse (x: '``Collection<'T>``, e: 'T, [<Optional>]_impl: Default1) =
+       x
+
+   static member Intersperse (x: list<'T>, e: 'T, [<Optional>]_impl: Intersperse) =
+       x
+
+   static member inline Invoke (sep: 'T) (source: '``Collection<'T>``) =
+       let inline call_2 (a: ^a, b: ^b, s) =
+           ((^a or ^b): (static member Intersperse: _ * _ * _ -> _) (b, s, a))
+
+       let inline call (a: 'a, b: 'b, s) =
+           call_2 (a, b, s)
+
+       call (Unchecked.defaultof<Intersperse>, source, sep) : '``Collection<'T>``
+
+let _ = Intersperse.Invoke 0 [1]
+"""
+       |> assertCompiles
+
+   [<Fact>]
+   let ``Issue 1565 example 1 compiles`` () =
+       FSharp """
+module Issue1565Example1
+
+let inline checkBounds f (g: 'b -> ^c) (tp: ^a) =
+   let convertFrom = (^a: (static member name: string) ())
+   let convertTo = (^c: (static member name : string) ())
+   let value = (^a: (member Value: 'b) tp)
+
+   if f value then
+       g value
+   else
+       failwithf "Cannot convert from %s to %s." convertFrom convertTo
+
+[<Struct>]
+type ConverterA =
+   val Value: sbyte
+   new(v) = { Value = v }
+
+   static member inline name with get () = "converter-a"
+
+   static member inline convert(x: ConverterA): ConverterB =
+       checkBounds ((>=) 0y) (byte >> ConverterB) x
+
+and [<Struct>] ConverterB =
+   val Value: byte
+   new(v) = { Value = v }
+
+   static member inline name with get () = "converter-b"
+"""
+       |> assertCompiles
+
+   [<Fact>]
+   let ``Issue 1565 example 2 compiles`` () =
+       FSharp """
+module Issue1565Example2
+
+[<System.Flags>]
+type MyType =
+   | Integer = 0b0001
+   | Float = 0b0010
+
+module Test =
+   [<CustomEquality; NoComparison>]
+   type SomeType =
+       | Int of int64
+       | Float of float
+
+       override x.Equals other =
+           match other with
+           | :? SomeType as y ->
+               match SomeType.getType x &&& SomeType.getType y with
+               | MyType.Integer -> int64 x = int64 y
+               | MyType.Float -> float x = float y
+               | _ -> false
+           | _ -> false
+
+       override x.GetHashCode() =
+           match x with
+           | Int i -> hash i
+           | Float f -> hash f
+
+       static member inline op_Explicit(n: SomeType): float =
+           match n with
+           | Int i -> float i
+           | Float f -> f
+
+       static member inline op_Explicit(n: SomeType): int64 =
+           match n with
+           | Int i -> i
+           | Float f -> int64 f
+
+       static member inline getType x =
+           match x with
+           | Int _ -> MyType.Integer
+           | Float _ -> MyType.Float
+"""
+       |> assertCompiles
+
+   [<Fact>]
+   let ``Issue 1565 example 3 compiles`` () =
+       FSharp """
+module Test
+
+type SomeType =
+   | Int of int64
+   | Float of float
+
+   static member MyEquals(x, other: SomeType) =
+       float x = float other
+
+   static member inline op_Explicit(n: SomeType): float =
+       match n with
+       | Int i -> float i
+       | Float f -> f
+
+   static member inline op_Explicit(n: SomeType): int64 =
+       match n with
+       | Int i -> i
+       | Float f -> int64 f
+"""
+       |> assertCompiles