Debugger: prefer no inlining in non-optimized builds (#19548)

Author: auduchinok

azure-pipelines-PR.yml

@@ -898,6 +898,6 @@ stages:
           buildScript: dotnet build Prime.sln --configuration Release
           displayName: Prime_Build
         - repo: bryanedds/Nu
-          commit: b321cb41a0bea0dab6b4509f895e6cd4d024e9e5
+          commit: e81e00a464b9d35d272f61708a1a0bfbf487b6d5
           buildScript: dotnet build Nu.sln --configuration Release
           displayName: Nu_Build

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

@@ -130,3 +130,4 @@
 * Exception field serialization (`GetObjectData` and field-restoring constructor) is now gated behind `langversion:11` (`LanguageFeature.ExceptionFieldSerializationSupport`). With langversion ≤10, exception codegen is unchanged from pre-#19342 behavior. ([PR #19746](https://github.com/dotnet/fsharp/pull/19746))
 
 ### Breaking Changes
+* Optimizer: don't inline named functions in debug builds ([PR #19548](https://github.com/dotnet/fsharp/pull/19548)

src/Compiler/Checking/PostInferenceChecks.fs

@@ -223,7 +223,9 @@ type cenv =
       mutable entryPointGiven: bool
 
       /// Callback required for quotation generation
-      tcVal: ConstraintSolver.TcValF }
+      tcVal: ConstraintSolver.TcValF
+
+      inlineBindingBodies: Dictionary<Stamp, Expr> }
 
     override x.ToString() = "<cenv>"
 
@@ -519,6 +521,9 @@ let AccessInternalsVisibleToAsInternal thisCompPath internalsVisibleToPaths acce
     (access, internalsVisibleToPaths) ||> List.fold (fun access internalsVisibleToPath ->
         accessSubstPaths (thisCompPath, internalsVisibleToPath) access)
 
+let isLessAccessibleWithVisibility (cenv: cenv) itemAccess refAccess =
+    let thisCompPath = compPathOfCcu cenv.viewCcu
+    isLessAccessible (itemAccess |> AccessInternalsVisibleToAsInternal thisCompPath cenv.internalsVisibleToPaths) refAccess
 
 let CheckTypeForAccess (cenv: cenv) env objName valAcc m ty =
     if cenv.reportErrors then
@@ -529,9 +534,7 @@ let CheckTypeForAccess (cenv: cenv) env objName valAcc m ty =
             match tryTcrefOfAppTy cenv.g ty with
             | ValueNone -> ()
             | ValueSome tcref ->
-                let thisCompPath = compPathOfCcu cenv.viewCcu
-                let tyconAcc = tcref.Accessibility |> AccessInternalsVisibleToAsInternal thisCompPath cenv.internalsVisibleToPaths
-                if isLessAccessible tyconAcc valAcc then
+                if isLessAccessibleWithVisibility cenv tcref.Accessibility valAcc then
                     errorR(Error(FSComp.SR.chkTypeLessAccessibleThanType(tcref.DisplayName, objName()), m))
 
         CheckTypeDeep cenv (visitType, None, None, None, None) cenv.g env NoInfo ty
@@ -545,9 +548,7 @@ let WarnOnWrongTypeForAccess (cenv: cenv) env objName valAcc m ty =
             match tryTcrefOfAppTy cenv.g ty with
             | ValueNone -> ()
             | ValueSome tcref ->
-                let thisCompPath = compPathOfCcu cenv.viewCcu
-                let tyconAcc = tcref.Accessibility |> AccessInternalsVisibleToAsInternal thisCompPath cenv.internalsVisibleToPaths
-                if isLessAccessible tyconAcc valAcc then
+                if isLessAccessibleWithVisibility cenv tcref.Accessibility valAcc then
                     let errorText = FSComp.SR.chkTypeLessAccessibleThanType(tcref.DisplayName, objName()) |> snd
                     let warningText = errorText + Environment.NewLine + FSComp.SR.tcTypeAbbreviationsCheckedAtCompileTime()
                     warning(ObsoleteDiagnostic(false, None, Some warningText, None, m))
@@ -2081,6 +2082,29 @@ and AdjustAccess isHidden (cpath: unit -> CompilationPath) access =
     else
         access
 
+// An 'inline' value is inlined into (possibly external) callers, so any function it references must be
+// at least as accessible as the value itself (FS1113). Inline callees are followed transitively because
+// the optimizer inlines them away; only module/member bindings can escape their scope.
+and CheckInlineValueIsSufficientlyAccessible cenv env (v: Val) bindRhs =
+    if cenv.reportErrors && v.ShouldInline && not v.IsCompilerGenerated &&
+       (v.IsMemberOrModuleBinding || v.IsMember) && not v.IsIncrClassGeneratedMember then
+        let inlineAcc =
+            AdjustAccess (IsHiddenVal env.sigToImplRemapInfo v) (fun () -> v.DeclaringEntity.CompilationPath) v.Accessibility
+        let visited = HashSet<Stamp>()
+        let rec escapes expr =
+            (freeInExpr CollectLocals expr).FreeLocals |> Zset.exists (fun w ->
+                (w.IsMemberOrModuleBinding || w.IsMember) &&
+                isLessAccessibleWithVisibility cenv w.Accessibility inlineAcc &&
+                (if w.ShouldInline then
+                     visited.Add w.Stamp &&
+                     (match cenv.inlineBindingBodies.TryGetValue w.Stamp with
+                      | true, body -> escapes body
+                      | _ -> false)
+                 else
+                     true))
+        if escapes bindRhs then
+            errorR(Error(FSComp.SR.optValueMarkedInlineButIncomplete(v.DisplayName), v.Range))
+
 and CheckBinding cenv env alwaysCheckNoReraise ctxt (TBind(v, bindRhs, _) as bind) : Limit =
     let vref = mkLocalValRef v
     let g = cenv.g
@@ -2113,6 +2137,8 @@ and CheckBinding cenv env alwaysCheckNoReraise ctxt (TBind(v, bindRhs, _) as bin
         let access =  AdjustAccess (IsHiddenVal env.sigToImplRemapInfo v) (fun () -> v.DeclaringEntity.CompilationPath) v.Accessibility
         CheckTypeForAccess cenv env (fun () -> NicePrint.stringOfQualifiedValOrMember cenv.denv cenv.infoReader vref) access v.Range v.Type
 
+    CheckInlineValueIsSufficientlyAccessible cenv env v bindRhs
+
     if cenv.reportErrors  then
 
         // Check top-level let-bound values
@@ -2781,7 +2807,20 @@ let CheckImplFileContents cenv env implFileTy implFileContents  =
     UpdatePrettyTyparNames.updateModuleOrNamespaceType implFileTy
     CheckDefnInModule cenv env implFileContents
 
+let rec private collectInlineBindingBodies (acc: Dictionary<Stamp, Expr>) mdef =
+    match mdef with
+    | TMDefRec(bindings = mbinds) ->
+        for mbind in mbinds do
+            match mbind with
+            | ModuleOrNamespaceBinding.Binding (TBind(v, e, _)) -> if v.ShouldInline then acc[v.Stamp] <- e
+            | ModuleOrNamespaceBinding.Module(_, def) -> collectInlineBindingBodies acc def
+    | TMDefLet(TBind(v, e, _), _) -> if v.ShouldInline then acc[v.Stamp] <- e
+    | TMDefDo _ | TMDefOpens _ -> ()
+    | TMDefs defs -> for def in defs do collectInlineBindingBodies acc def
+
 let CheckImplFile (g, amap, reportErrors, infoReader, internalsVisibleToPaths, viewCcu, tcValF, denv, implFileTy, implFileContents, extraAttribs, isLastCompiland: bool*bool, isInternalTestSpanStackReferring) =
+    let inlineBindingBodies = Dictionary<Stamp, Expr>(HashIdentity.Structural)
+    collectInlineBindingBodies inlineBindingBodies implFileContents
     let cenv =
         { g = g
           reportErrors = reportErrors
@@ -2799,7 +2838,8 @@ let CheckImplFile (g, amap, reportErrors, infoReader, internalsVisibleToPaths, v
           isLastCompiland = isLastCompiland
           isInternalTestSpanStackReferring = isInternalTestSpanStackReferring
           tcVal = tcValF
-          entryPointGiven = false}
+          entryPointGiven = false
+          inlineBindingBodies = inlineBindingBodies }
 
     // Certain type equality checks go faster if these TyconRefs are pre-resolved.
     // This is because pre-resolving allows tycon equality to be determined by pointer equality on the entities.

src/Compiler/CodeGen/IlxGen.fs

@@ -269,6 +269,8 @@ type IlxGenOptions =
 
         /// When set to true, the IlxGen will delay generation of method bodies and generated them later in parallel (parallelized across files)
         parallelIlxGenEnabled: bool
+
+        alwaysInline: bool
     }
 
 /// Compilation environment for compiling a fragment of an assembly
@@ -5840,8 +5842,13 @@ and GenTraitCall (cenv: cenv) cgbuf eenv (traitInfo: TraitConstraintInfo, argExp
 
     | None ->
 
-        // If witnesses are available, we should now always find trait witnesses in scope
-        assert not generateWitnesses
+        // When alwaysInline is true, all trait calls should be resolved via witnesses in scope.
+        // When alwaysInline is false, inline functions are kept as calls rather than inlined.
+        // Their witness arguments may contain TraitCall operations for constraints that were resolved
+        // without a witness (e.g., when the constraint is satisfied by a known concrete type).
+        // In such cases, generateWitnesses can be true (because other witnesses are in scope) but
+        // the specific trait's witness is not found. Fall through to the constraint solver to resolve it.
+        assert (not generateWitnesses || not cenv.options.alwaysInline)
 
         let exprOpt =
             CommitOperationResult(ConstraintSolver.CodegenWitnessExprForTraitConstraint cenv.tcVal g cenv.amap m traitInfo argExprs)
@@ -7290,8 +7297,19 @@ and GetIlxClosureFreeVars cenv m (thisVars: ValRef list) boxity eenv takenNames
         let cloName =
             // Ensure that we have an g.CompilerGlobalState
             assert (g.CompilerGlobalState |> Option.isSome)
+            // The closure name counter is keyed by (basicName, fileIndex). When an expression is copied
+            // from another file (e.g. specializing an inline function body across files), its ranges
+            // still point at the original file, so its closures fall into a different counter bucket
+            // than closures minted for the current file. Since all these closures live under the same
+            // enclosing type, that can produce two closures with the same final name. Bucket the counter
+            // by the enclosing type's file while keeping expr.Range's StartLine for the displayed name.
+            let nameRange =
+                if expr.Range.FileIndex = eenv.cloc.Range.FileIndex then
+                    expr.Range
+                else
+                    Range.mkFileIndexRange eenv.cloc.Range.FileIndex expr.Range.Start expr.Range.End
 
-            g.CompilerGlobalState.Value.StableNameGenerator.GetUniqueCompilerGeneratedName(basenameSafeForUseAsTypename, expr.Range, uniq)
+            g.CompilerGlobalState.Value.StableNameGenerator.GetUniqueCompilerGeneratedName(basenameSafeForUseAsTypename, nameRange, uniq)
 
         let ilCloTypeRef = NestedTypeRefForCompLoc eenv.cloc cloName
 
@@ -7349,6 +7367,21 @@ and GetIlxClosureFreeVars cenv m (thisVars: ValRef list) boxity eenv takenNames
 
     let cloFreeTyvars = cloFreeTyvars.FreeTypars |> Zset.elements
 
+    // When generating witnesses, witness types may reference type variables that appear
+    // only in SRTP constraints of the captured type variables (e.g. 'b in 'a : (member M: unit -> 'b)).
+    // Include those so they are available when generating witness field types.
+    let cloFreeTyvars =
+        if ComputeGenerateWitnesses g eenv then
+            let extra =
+                GetTraitWitnessInfosOfTypars g 0 cloFreeTyvars
+                |> List.collect (fun w ->
+                    (freeInType CollectTyparsNoCaching (GenWitnessTy g w)).FreeTypars
+                    |> Zset.elements)
+
+            (cloFreeTyvars @ extra) |> List.distinctBy (fun tp -> tp.Stamp)
+        else
+            cloFreeTyvars
+
     let eenvinner = eenv |> EnvForTypars cloFreeTyvars
 
     let ilCloTyInner =

src/Compiler/CodeGen/IlxGen.fsi

@@ -61,6 +61,9 @@ type internal IlxGenOptions =
 
         /// When set to true, the IlxGen will delay generation of method bodies and generate them later in parallel (parallelized across files)
         parallelIlxGenEnabled: bool
+
+        /// Indicates if inline functions are being inlined or emitted as calls
+        alwaysInline: bool
     }
 
 /// The results of the ILX compilation of one fragment of an assembly

src/Compiler/Driver/CompilerConfig.fs

@@ -600,6 +600,8 @@ type TcConfigBuilder =
 
         mutable strictIndentation: bool option
 
+        mutable alwaysInline: bool option
+
         mutable exename: string option
 
         // If true - the compiler will copy FSharp.Core.dll along the produced binaries
@@ -853,6 +855,7 @@ type TcConfigBuilder =
             dumpSignatureData = false
             realsig = false
             strictIndentation = None
+            alwaysInline = None
             compilationMode = TcGlobals.CompilationMode.Unset
         }
 
@@ -1253,6 +1256,14 @@ type TcConfig private (data: TcConfigBuilder, validate: bool) =
     member _.fsiMultiAssemblyEmit = data.fsiMultiAssemblyEmit
     member _.FxResolver = data.FxResolver
     member _.strictIndentation = data.strictIndentation
+
+    member _.alwaysInline =
+        data.alwaysInline
+        |> Option.defaultValue (
+            data.optSettings.LocalOptimizationsEnabled
+            || data.extraOptimizationIterations > 0
+        )
+
     member _.primaryAssembly = data.primaryAssembly
     member _.noFeedback = data.noFeedback
     member _.stackReserveSize = data.stackReserveSize

src/Compiler/Driver/CompilerConfig.fsi

@@ -472,6 +472,8 @@ type TcConfigBuilder =
 
         mutable strictIndentation: bool option
 
+        mutable alwaysInline: bool option
+
         mutable exename: string option
 
         mutable copyFSharpCore: CopyFSharpCoreFlag
@@ -814,6 +816,8 @@ type TcConfig =
 
     member strictIndentation: bool option
 
+    member alwaysInline: bool
+
     member GetTargetFrameworkDirectories: unit -> string list
 
     /// Get the loaded sources that exist and issue a warning for the ones that don't

src/Compiler/Driver/CompilerOptions.fs

@@ -1207,6 +1207,14 @@ let languageFlags tcConfigB =
             None,
             Some(FSComp.SR.optsStrictIndentation (formatOptionSwitch (Option.defaultValue false tcConfigB.strictIndentation)))
         )
+
+        CompilerOption(
+            "always-inline",
+            tagNone,
+            OptionSwitch(fun switch -> tcConfigB.alwaysInline <- Some(switch = OptionSwitch.On)),
+            None,
+            Some(FSComp.SR.optsAlwaysInline ())
+        )
     ]
 
 // OptionBlock: Advanced user options

src/Compiler/Driver/OptimizeInputs.fs

@@ -327,6 +327,7 @@ let ApplyAllOptimizations
             // Only do abstractBigTargets in the first phase, and only when TLR is on.
             abstractBigTargets = tcConfig.doTLR
             reportingPhase = true
+            alwaysInline = tcConfig.alwaysInline
         }
 
     // Only do these two steps in the first phase.
@@ -583,6 +584,7 @@ let GenerateIlxCode
             isInteractiveItExpr = isInteractiveItExpr
             alwaysCallVirt = tcConfig.alwaysCallVirt
             parallelIlxGenEnabled = tcConfig.parallelIlxGen
+            alwaysInline = tcConfig.alwaysInline
         }
 
     ilxGenerator.GenerateCode(ilxGenOpts, optimizedImpls, topAttrs.assemblyAttrs, topAttrs.netModuleAttrs)

src/Compiler/FSComp.txt

@@ -1561,6 +1561,7 @@ optsSetLangVersion,"Specify language version such as 'latest' or 'preview'."
 optsDisableLanguageFeature,"Disable a specific language feature by name."
 optsSupportedLangVersions,"Supported language versions:"
 optsStrictIndentation,"Override indentation rules implied by the language version (%s by default)"
+optsAlwaysInline,"Always inline 'inline' functions"
 nativeResourceFormatError,"Stream does not begin with a null resource and is not in '.RES' format."
 nativeResourceHeaderMalformed,"Resource header beginning at offset %s is malformed."
 formatDashItem," - %s"