Merge pull request #17201 from vzarytovskii/fix-inlining-regression

Fix inlining regression

Author: vzarytovskii

eng/Versions.props

@@ -14,7 +14,7 @@
     <!-- F# Version components -->
     <FSMajorVersion>8</FSMajorVersion>
     <FSMinorVersion>0</FSMinorVersion>
-    <FSBuildVersion>300</FSBuildVersion>
+    <FSBuildVersion>301</FSBuildVersion>
     <FSRevisionVersion>0</FSRevisionVersion>
     <!-- -->
     <!-- F# Language version -->

global.json

@@ -14,7 +14,7 @@
     "xcopy-msbuild": "17.8.1-2"
   },
   "native-tools": {
-    "perl": "5.38.0.1"
+    "perl": "5.38.2.2"
   },
   "msbuild-sdks": {
     "Microsoft.DotNet.Arcade.Sdk": "8.0.0-beta.24204.3",

src/Compiler/Optimize/Optimizer.fs

@@ -2356,15 +2356,7 @@ let rec OptimizeExpr cenv (env: IncrementalOptimizationEnv) expr =
         OptimizeConst cenv env expr (c, m, ty)
 
     | Expr.Val (v, _vFlags, m) ->
-        if not (v.Accessibility.IsPrivate) then
-            OptimizeVal cenv env expr (v, m)
-        else
-            expr,
-            { TotalSize = 10
-              FunctionSize = 1
-              HasEffect = false  
-              MightMakeCriticalTailcall=false
-              Info=UnknownValue }
+        OptimizeVal cenv env expr (v, m)
 
 
     | Expr.Quote (ast, splices, isFromQueryExpression, m, ty) -> 
@@ -3082,9 +3074,6 @@ and TryOptimizeVal cenv env (vOpt: ValRef option, shouldInline, inlineIfLambda,
         let fvs = freeInExpr CollectLocals expr
         if fvs.UsesMethodLocalConstructs then
             // Discarding lambda for binding because uses protected members --- TBD: Should we warn or error here
-            None 
-        elif fvs.FreeLocals |> Seq.exists(fun v -> v.Accessibility.IsPrivate ) then
-            // Discarding lambda for binding because uses private members --- TBD: Should we warn or error here
             None
         else
             let exprCopy = CopyExprForInlining cenv inlineIfLambda expr m
@@ -4123,10 +4112,10 @@ and OptimizeBinding cenv isRec env (TBind(vref, expr, spBind)) =
                     let fvs = freeInExpr CollectLocals body
                     if fvs.UsesMethodLocalConstructs then
                         // Discarding lambda for binding because uses protected members
-                        UnknownValue 
+                        UnknownValue
                     elif fvs.FreeLocals.ToArray() |> Seq.fold(fun acc v -> if not acc then v.Accessibility.IsPrivate else acc) false then
                         // Discarding lambda for binding because uses private members
-                        UnknownValue 
+                        UnknownValue
                     else
                         ivalue
 

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

@@ -16,6 +16,10 @@ module Inlining =
         |> ignoreWarnings
         |> verifyILBaseline
 
+    let withRealInternalSignature realSig compilation =
+        compilation
+        |> withOptions [if realSig then "--realsig+" else "--realsig-" ]
+
     // SOURCE=Match01.fs SCFLAGS="-a --optimize+" COMPILE_ONLY=1 POSTCMD="..\\CompareIL.cmd Match01.dll"	# Match01.fs
     [<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"Match01_RealInternalSignatureOn.fs"|])>]
     let ``Match01_RealInternalSignatureOn_fs`` compilation =
@@ -139,3 +143,82 @@ let found = data |> List.contains nan
   }"""]
 #endif
 
+    [<InlineData(true)>]        // RealSig
+    [<InlineData(false)>]       // Regular
+    [<Theory>]
+    let ``Inlining field with private module`` (realSig) =
+        Fsx """
+module private PrivateModule =
+    let moduleValue = 1
+
+    let inline getModuleValue () =
+        moduleValue
+
+[<EntryPoint>]
+let main argv =
+    //   [FS1118] Failed to inline the value 'getModuleValue' marked 'inline', perhaps because a recursive value was marked 'inline'
+    //   (fixed by making PrivateModule internal instead of private)
+    PrivateModule.getModuleValue () |> ignore
+    0
+            """
+        |> withOptimize
+        |> withRealInternalSignature realSig
+        |> asExe
+        |> compileAndRun
+        |> shouldSucceed
+
+    [<InlineData(true)>]        // RealSig
+    [<InlineData(false)>]       // Regular
+    [<Theory>]
+    let ``Inlining field with private class`` (realSig) =
+        Fsx """
+type private FirstType () =
+    member this.FirstMethod () = ()
+
+type private SecondType () =
+    member this.SecondMethod () =
+        let inline callFirstMethod (first: FirstType) =
+            first.FirstMethod ()
+
+        callFirstMethod (FirstType())
+
+printfn $"{(SecondType ()).SecondMethod()}"
+            """
+        |> withOptimize
+        |> withRealInternalSignature realSig
+        |> asExe
+        |> compileAndRun
+        |> shouldSucceed
+
+    [<InlineData(true)>]        // RealSig
+    [<InlineData(false)>]       // Regular
+    [<Theory>]
+    let ``Inlining deep local functions field with private class`` (realSig) =
+        Fsx """
+type private FirstType () =
+    member this.FirstMethod () = ()
+
+type private SecondType () =
+    member this.SecondMethod () =
+        let inline callFirstMethod (first: FirstType) =
+            first.FirstMethod ()
+
+        let inline callFirstMethodDeeper (first: FirstType) =
+            callFirstMethod (first)
+
+        let inline callFirstMethodMoreDeeper (first: FirstType) =
+            callFirstMethodDeeper (first)
+
+        let inline callFirstMethodMostDeeply (first: FirstType) =
+            callFirstMethodMoreDeeper (first)
+
+        callFirstMethodMostDeeply (FirstType())
+
+printfn $"{(SecondType ()).SecondMethod()}"
+            """
+        |> withOptimize
+        |> withRealInternalSignature realSig
+        |> asExe
+        |> compileAndRun
+        |> shouldSucceed
+