diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index b7b9889a82f..6445d1ba2a8 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -7,6 +7,7 @@ * Diagnostic FS0027 now emits a parameter-specific message (suggesting a `let mutable x = x` shadow or `byref<_>`) instead of the illegal `let mutable x = expression` shadow when the assignment target is a function or method parameter. ([Issue #15803](https://github.com/dotnet/fsharp/issues/15803), [PR #19866](https://github.com/dotnet/fsharp/pull/19866)) * Recursive `inline` functions and members now emit a single clear error (FS3890) instead of a misleading FS1113/FS1114 optimizer cascade. ([Issue #17991](https://github.com/dotnet/fsharp/issues/17991), [PR #19803](https://github.com/dotnet/fsharp/pull/19803)) * Report `FS0037 Duplicate definition of type or module` at type-check time when two sibling modules in a `module rec` / `namespace rec` group share a name, instead of letting the duplicate slip through to IL emit where it surfaced as the cryptic `FS2014: duplicate entry . in type index table`. ([Issue #6694](https://github.com/dotnet/fsharp/issues/6694), [PR #19913](https://github.com/dotnet/fsharp/pull/19913)) +* Unused `Unchecked.defaultof<'T>` bindings are now eliminated under optimization at their use sites. This removes redundant `initobj`/`ldnull;pop` sequences left behind after inlining SRTP helpers that use `Unchecked.defaultof` as dummy arguments to drive static-member-constraint resolution. Such bindings are preserved inside `inline` bodies (whose optimized form is pickled as cross-assembly optimization info and re-inlined by consumers) and whenever the erased type still references unsolved type variables, so no `FS0073` regression is introduced. ([Issue #18128](https://github.com/dotnet/fsharp/issues/18128), [PR #19758](https://github.com/dotnet/fsharp/pull/19758)) * Semantic classification no longer marks recursive object self-references (`as this`, `let rec` self-refs) as mutable. ([Issue #5229](https://github.com/dotnet/fsharp/issues/5229)) * Fix `MethodAccessException` under `--realsig+` when a closure (inner `let rec`, `task`/`async` state machine, or quotation splice) inside a member defined in an intrinsic type augmentation (`type C with member ...`) accesses a `private` member of `C`. The synthesized closure is now nested inside the declaring type instead of beside it in the module class. ([Issue #19933](https://github.com/dotnet/fsharp/issues/19933), [PR #19955](https://github.com/dotnet/fsharp/pull/19955)) * Preserve source range for type errors on empty-bodied computation expressions (e.g. `foo {}`) in pipelines, function arguments, and type-annotated contexts, instead of reporting `unknown(1,1)`. ([Issue #19550](https://github.com/dotnet/fsharp/issues/19550), [PR #19849](https://github.com/dotnet/fsharp/pull/19849)) diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs index c7100cd69aa..72324db79a2 100644 --- a/src/Compiler/CodeGen/IlxGen.fs +++ b/src/Compiler/CodeGen/IlxGen.fs @@ -4706,7 +4706,7 @@ and GenApp (cenv: cenv) cgbuf eenv (f, fty, tyargs, curriedArgs, m) sequel = (eenv, laterArgs) ||> List.mapFold (fun eenv laterArg -> // Only save arguments that have effects - if Optimizer.ExprHasEffect g laterArg then + if Optimizer.ExprHasEffect Optimizer.EffectContext.Emit g laterArg then let ilTy = laterArg |> tyOfExpr g |> GenType cenv m eenv.tyenv let locName = diff --git a/src/Compiler/Optimize/Optimizer.fs b/src/Compiler/Optimize/Optimizer.fs index 51f53ff01ae..8f3a550bdb7 100644 --- a/src/Compiler/Optimize/Optimizer.fs +++ b/src/Compiler/Optimize/Optimizer.fs @@ -1616,15 +1616,47 @@ let SplitValuesByIsUsedOrHasEffect cenv fvs x = let IlAssemblyCodeInstrHasEffect i = match i with - | ( AI_nop | AI_ldc _ | AI_add | AI_sub | AI_mul | AI_xor | AI_and | AI_or - | AI_ceq | AI_cgt | AI_cgt_un | AI_clt | AI_clt_un | AI_conv _ | AI_shl - | AI_shr | AI_shr_un | AI_neg | AI_not | AI_ldnull ) + | AI_nop | AI_ldc _ | AI_add | AI_sub | AI_mul | AI_xor | AI_and | AI_or + | AI_ceq | AI_cgt | AI_cgt_un | AI_clt | AI_clt_un | AI_conv _ | AI_shl + | AI_shr | AI_shr_un | AI_neg | AI_not | AI_ldnull | I_ldstr _ | I_ldtoken _ -> false | _ -> true - + let IlAssemblyCodeHasEffect instrs = List.exists IlAssemblyCodeInstrHasEffect instrs -let rec ExprHasEffect g expr = +/// The context an expression's effects are analyzed in. It governs whether a fully-ground +/// `Unchecked.defaultof` (an `EI_ilzero` witness) may be treated as effect-free and its unused +/// binding dropped: safe when emitting here, but not inside an `inline` value's body, whose +/// optimized form is pickled as cross-assembly info and re-inlined at each use site — dropping +/// the witness there corrupts that info and trips FS0073 in the consumer's IlxGen (e.g. the SRTP +/// witness/dummy arguments used pervasively by FSharpPlus). +[] +type EffectContext = + /// Emitting IL here, at a fully instantiated use site. + | Emit + /// Analyzing the pickled body of an `inline` value, re-inlined at other sites. + | InlineBody + +let ILAsmWithIlzeroHasEffect context instrs tyargs = + let hasIlzero = instrs |> List.exists (function EI_ilzero _ -> true | _ -> false) + + if not hasIlzero then + IlAssemblyCodeHasEffect instrs + else + // A fully-ground ilzero is a pure default-value load whose unused binding may be dropped, + // but only at emission and only when no tyarg still holds a free typar: such a binding is + // the sole pin for those typars and dropping it leaves them unsolved (FS0073). + let ilzeroIsSafeToDrop = + match context with + | EffectContext.Emit -> tyargs |> List.forall (fun ty -> Zset.isEmpty (freeInType CollectTypars ty).FreeTypars) + | EffectContext.InlineBody -> false + + let otherInstrsHaveEffect = + instrs |> List.exists (function EI_ilzero _ -> false | i -> IlAssemblyCodeInstrHasEffect i) + + otherInstrsHaveEffect || not ilzeroIsSafeToDrop + +let rec ExprHasEffect context g expr = match stripDebugPoints expr with | Expr.Val (vref, _, _) -> vref.IsTypeFunction || vref.IsMutable | Expr.Quote _ @@ -1632,20 +1664,20 @@ let rec ExprHasEffect g expr = | Expr.TyLambda _ | Expr.Const _ -> false // type applications do not have effects, with the exception of type functions - | Expr.App (f0, _, _, [], _) -> IsTyFuncValRefExpr f0 || ExprHasEffect g f0 - | Expr.Op (op, _, args, m) -> ExprsHaveEffect g args || OpHasEffect g m op - | Expr.LetRec (binds, body, _, _) -> BindingsHaveEffect g binds || ExprHasEffect g body - | Expr.Let (bind, body, _, _) -> BindingHasEffect g bind || ExprHasEffect g body + | Expr.App (f0, _, _, [], _) -> IsTyFuncValRefExpr f0 || ExprHasEffect context g f0 + | Expr.Op (op, tyargs, args, m) -> ExprsHaveEffect context g args || OpHasEffect context g m op tyargs + | Expr.LetRec (binds, body, _, _) -> BindingsHaveEffect context g binds || ExprHasEffect context g body + | Expr.Let (bind, body, _, _) -> BindingHasEffect context g bind || ExprHasEffect context g body // REVIEW: could add Expr.Obj on an interface type - these are similar to records of lambda expressions | _ -> true -and ExprsHaveEffect g exprs = List.exists (ExprHasEffect g) exprs +and ExprsHaveEffect context g exprs = List.exists (ExprHasEffect context g) exprs -and BindingsHaveEffect g binds = List.exists (BindingHasEffect g) binds +and BindingsHaveEffect context g binds = List.exists (BindingHasEffect context g) binds -and BindingHasEffect g bind = bind.Expr |> ExprHasEffect g +and BindingHasEffect context g bind = bind.Expr |> ExprHasEffect context g -and OpHasEffect g m op = +and OpHasEffect context g m op tyargs = match op with | TOp.Tuple _ -> false | TOp.AnonRecd _ -> false @@ -1659,7 +1691,7 @@ and OpHasEffect g m op = | TOp.UnionCaseTagGet _ -> false | TOp.UnionCaseProof _ -> false | TOp.UnionCaseFieldGet (ucref, n) -> isUnionCaseFieldMutable g ucref n - | TOp.ILAsm (instrs, _) -> IlAssemblyCodeHasEffect instrs + | TOp.ILAsm (instrs, _) -> ILAsmWithIlzeroHasEffect context instrs tyargs | TOp.TupleFieldGet _ -> false | TOp.ExnFieldGet (ecref, n) -> isExnFieldMutable ecref n | TOp.RefAddrGet _ -> false @@ -1686,6 +1718,9 @@ and OpHasEffect g m op = | TOp.LValueOp _ (* conservative *) | TOp.ValFieldSet _ -> true +let effectContextOf (cenv: cenv) = + if cenv.optimizing then EffectContext.Emit else EffectContext.InlineBody + let TryEliminateBinding cenv _env bind e2 _m = let g = cenv.g @@ -1715,7 +1750,7 @@ let TryEliminateBinding cenv _env bind e2 _m = match argsr with | Expr.Val (VRefLocal vspec2, _, _) :: argsr2 when valEq vspec1 vspec2 && IsUniqueUse vspec2 (List.rev rargsl@argsr2) -> Some(List.rev rargsl, argsr2) - | argsrh :: argsrt when not (ExprHasEffect g argsrh) -> GetImmediateUseContext (argsrh :: rargsl) argsrt + | argsrh :: argsrt when not (ExprHasEffect (effectContextOf cenv) g argsrh) -> GetImmediateUseContext (argsrh :: rargsl) argsrt | _ -> None let (DebugPoints(e2, recreate0)) = e2 @@ -2590,7 +2625,7 @@ and OptimizeExprOp cenv env (op, tyargs, args, m) = newExpr, { TotalSize = 1 FunctionSize = 1 - HasEffect = OpHasEffect g m newOp + HasEffect = OpHasEffect (effectContextOf cenv) g m newOp tyargs MightMakeCriticalTailcall = false Info = ValueOfExpr newExpr } @@ -2667,7 +2702,7 @@ and OptimizeExprOpFallback cenv env (op, tyargs, argsR, m) arginfos value_ = let argsFSize = AddFunctionSizes arginfos let argEffects = OrEffects arginfos let argValues = List.map (fun x -> x.Info) arginfos - let effect = OpHasEffect g m op + let effect = OpHasEffect (effectContextOf cenv) g m op tyargs let cost, value_ = match op with | TOp.UnionCase c -> 2, MakeValueInfoForUnionCase c (Array.ofList argValues) diff --git a/src/Compiler/Optimize/Optimizer.fsi b/src/Compiler/Optimize/Optimizer.fsi index 17912af7598..8ec9b556ca2 100644 --- a/src/Compiler/Optimize/Optimizer.fsi +++ b/src/Compiler/Optimize/Optimizer.fsi @@ -108,8 +108,16 @@ val AbstractOptimizationInfoToEssentials: (CcuOptimizationInfo -> CcuOptimizatio /// Combine optimization infos val UnionOptimizationInfos: seq -> CcuOptimizationInfo +/// The context an expression's effects are analyzed in: emitting IL here, or analyzing the +/// pickled body of an `inline` value. Governs whether a fully-ground `Unchecked.defaultof` +/// binding may be eliminated. +[] +type EffectContext = + | Emit + | InlineBody + /// Check if an expression has an effect -val ExprHasEffect: TcGlobals -> Expr -> bool +val ExprHasEffect: EffectContext -> TcGlobals -> Expr -> bool val internal u_CcuOptimizationInfo: ReaderState -> CcuOptimizationInfo diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/UncheckedDefaultofOptimization.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/UncheckedDefaultofOptimization.fs new file mode 100644 index 00000000000..43688b29c5c --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/UncheckedDefaultofOptimization.fs @@ -0,0 +1,126 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace EmittedIL + +open Xunit +open FSharp.Test.Compiler + +module ``UncheckedDefaultofOptimization`` = + + // https://github.com/dotnet/fsharp/issues/18128 + // Unused `Unchecked.defaultof` bindings should be eliminated under optimization. + // Pins both the absence of `initobj` and that no decimal local slot is allocated for any of the + // three discarded bindings. + [] + let ``Unused Unchecked.defaultof bindings of concrete types are eliminated`` () = + FSharp """ +module Test +open System +let f (n: float32) = + Console.WriteLine n + let _ = Unchecked.defaultof + let _ = Unchecked.defaultof + let _ = Unchecked.defaultof + let n' = n * 2.f + Console.WriteLine n' + """ + |> withOptimize + |> asLibrary + |> compile + |> shouldSucceed + |> verifyILNotPresent [ "initobj"; "valuetype [runtime]System.Decimal" ] + + // https://github.com/dotnet/fsharp/issues/18128 + // The FSharpPlus-style SRTP witness pattern from the issue. Elimination happens at the (fully + // instantiated) use site: `doWork` reduces to a direct multiplication with the `nil` and + // `nil< ^b >` witness bindings gone. The assertion is scoped to `doWork` rather than the whole + // module because the inline functions also emit a compiler-generated dynamic-invocation stub which + // legitimately retains an `ldnull` (see the guard below and issue #19758 - the witness bindings of + // an *inline* body must be preserved so cross-assembly consumers can re-inline and specialize them). + [] + let ``Unused Unchecked.defaultof SRTP witness bindings are eliminated at the use site`` () = + FSharp """ +module Test + +open System.ComponentModel + +[] +type PreOps = + static member inline Double (n: float<'u>) : float<'u> = n * 2. + static member inline Double (n: float32<'u>) : float32<'u> = n * 2.f + +#nowarn "64" +module PreludeOperators = + let inline private nil<'T> = Unchecked.defaultof<'T> + let inline double (x: ^a) = + let inline _call (_: ^M, input: ^I, _: ^R) = ((^M or ^I) : (static member Double : ^I -> ^R) input) + _call (nil, x, nil< ^b >) + +open PreludeOperators +let doWork (n: float) = double n + """ + |> withOptimize + |> asLibrary + |> compile + |> shouldSucceed + |> verifyIL [ + """.method public static float64 doWork(float64 n) cil managed + { + + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldc.r8 2. + IL_000a: mul + IL_000b: ret + }""" + ] + + // https://github.com/dotnet/fsharp/issues/18128 + // Soundness pin: eliminating an unused `Unchecked.defaultof` must not introduce a new reference + // to T in the enclosing method. `defaultof` of a reference type lowers to `ldnull` (not `newobj`), + // so removing the binding cannot suppress an observable cctor call - `f`'s body must contain no + // reference to `WithCctor` at all. + [] + let ``Eliminated Unchecked.defaultof leaves no reference to T in the caller`` () = + FSharp """ +module Test + +type WithCctor() = + static do failwith "cctor must not run" + +let f () = + let _ = Unchecked.defaultof + 42 + """ + |> withOptimize + |> asLibrary + |> compile + |> shouldSucceed + |> verifyIL [ + """.method public static int32 f() cil managed + { + + .maxstack 8 + IL_0000: ldc.i4.s 42 + IL_0002: ret + }""" + ] + + // https://github.com/dotnet/fsharp/issues/18128 + // Regression for SRTP witness/dummy-argument patterns (e.g. FSharpPlus) where eliminating an + // `Unchecked.defaultof` binding whose type still references unsolved typars would trip FS0073 in + // IlxGen. Such bindings must be kept; only fully-ground ilzero bindings are removed. + [] + let ``Unused Unchecked.defaultof bindings of unsolved generic types do not cause FS0073`` () = + FSharp """ +module Test +let inline witness () = Unchecked.defaultof<'T> +let inline run< ^T when ^T: (static member Zero: ^T)> () = + let _ = (witness () : ^T) + LanguagePrimitives.GenericZero< ^T> +let v : int = run< int> () + """ + |> withOptimize + |> asLibrary + |> compile + |> shouldSucceed diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj index 881ae13942b..4bd64396b11 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj +++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj @@ -250,6 +250,7 @@ +