From 0d598678272861dab2ffc3b846570b65fa1dffa7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 15:30:38 +0000 Subject: [PATCH 01/12] Initial plan From cff9c64f89e1c8a78fc2271d713c6aec1f6a4f1e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 16:07:56 +0000 Subject: [PATCH 02/12] Implement StringLengthZero optimization for empty string patterns Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- src/Compiler/Checking/FindUnsolved.fs | 1 + src/Compiler/Checking/PatternMatchCompilation.fs | 15 ++++++++++++++- src/Compiler/Checking/PostInferenceChecks.fs | 1 + src/Compiler/Checking/QuotationTranslator.fs | 9 +++++++++ src/Compiler/CodeGen/IlxGen.fs | 2 ++ src/Compiler/Symbols/Exprs.fs | 8 ++++++++ src/Compiler/TypedTree/TypedTree.fs | 3 +++ src/Compiler/TypedTree/TypedTree.fsi | 3 +++ src/Compiler/TypedTree/TypedTreeOps.fs | 3 +++ src/Compiler/TypedTree/TypedTreeOps.fsi | 2 ++ src/Compiler/TypedTree/TypedTreePickle.fs | 4 ++++ 11 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/Compiler/Checking/FindUnsolved.fs b/src/Compiler/Checking/FindUnsolved.fs index c1de629a07c..d3424a4bf6b 100644 --- a/src/Compiler/Checking/FindUnsolved.fs +++ b/src/Compiler/Checking/FindUnsolved.fs @@ -216,6 +216,7 @@ and accDiscrim cenv env d mFallback = | DecisionTreeTest.ArrayLength(_, ty) -> accTy cenv env mFallback ty | DecisionTreeTest.Const _ | DecisionTreeTest.IsNull -> () + | DecisionTreeTest.StringLengthZero ty -> accTy cenv env mFallback ty | DecisionTreeTest.IsInst (srcTy, tgtTy) -> accTy cenv env mFallback srcTy; accTy cenv env mFallback tgtTy | DecisionTreeTest.ActivePatternCase (exp, tys, _, _, _, _) -> accExpr cenv env exp diff --git a/src/Compiler/Checking/PatternMatchCompilation.fs b/src/Compiler/Checking/PatternMatchCompilation.fs index 0532182fb88..f816ffa3f7b 100644 --- a/src/Compiler/Checking/PatternMatchCompilation.fs +++ b/src/Compiler/Checking/PatternMatchCompilation.fs @@ -611,6 +611,8 @@ let getDiscrimOfPattern (g: TcGlobals) tpinst t = Some(DecisionTreeTest.IsInst (instType tpinst srcTy, instType tpinst tgtTy)) | TPat_exnconstr(tcref, _, _m) -> Some(DecisionTreeTest.IsInst (g.exn_ty, mkWoNullAppTy tcref [])) + | TPat_const (Const.String "", _m) -> + Some(DecisionTreeTest.StringLengthZero g.string_ty) | TPat_const (c, _m) -> Some(DecisionTreeTest.Const c) | TPat_unioncase (c, tyargs', _, _m) -> @@ -639,6 +641,7 @@ let discrimsEq (g: TcGlobals) d1 d2 = | DecisionTreeTest.ArrayLength (n1, _), DecisionTreeTest.ArrayLength(n2, _) -> (n1=n2) | DecisionTreeTest.Const c1, DecisionTreeTest.Const c2 -> (c1=c2) | DecisionTreeTest.IsNull, DecisionTreeTest.IsNull -> true + | DecisionTreeTest.StringLengthZero _, DecisionTreeTest.StringLengthZero _ -> true | DecisionTreeTest.IsInst (srcTy1, tgtTy1), DecisionTreeTest.IsInst (srcTy2, tgtTy2) -> typeEquiv g srcTy1 srcTy2 && typeEquiv g tgtTy1 tgtTy2 | DecisionTreeTest.ActivePatternCase (_, _, _, vrefOpt1, n1, _), DecisionTreeTest.ActivePatternCase (_, _, _, vrefOpt2, n2, _) -> match vrefOpt1, vrefOpt2 with @@ -658,6 +661,9 @@ let isDiscrimSubsumedBy g amap m discrim taken = computeWhatFailingNullTestImpliesAboutTypeTest g tgtTy2 = Implication.Fails | DecisionTreeTest.IsInst (_, tgtTy1), DecisionTreeTest.IsNull -> computeWhatFailingTypeTestImpliesAboutNullTest g tgtTy1 = Implication.Fails + | DecisionTreeTest.IsNull, DecisionTreeTest.StringLengthZero _ -> + // Null subsumes empty string - if null is already handled, we don't need a separate empty string case + true | _ -> false @@ -689,6 +695,7 @@ let discrimWithinSimultaneousClass g amap m discrim prev = match discrim, prev with | _, [] -> true | DecisionTreeTest.Const _, DecisionTreeTest.Const _ :: _ + | DecisionTreeTest.StringLengthZero _, DecisionTreeTest.StringLengthZero _ :: _ | DecisionTreeTest.ArrayLength _, DecisionTreeTest.ArrayLength _ :: _ | DecisionTreeTest.UnionCase _, DecisionTreeTest.UnionCase _ :: _ -> true @@ -791,12 +798,15 @@ let rec BuildSwitch inpExprOpt g expr edges dflt m = error(InternalError("inexhaustive match - need a default case!", m)) // Split string, float, uint64, int64, unativeint, nativeint matches into serial equality tests - | TCase((DecisionTreeTest.ArrayLength _ | DecisionTreeTest.Const (Const.Single _ | Const.Double _ | Const.String _ | Const.Decimal _ | Const.Int64 _ | Const.UInt64 _ | Const.IntPtr _ | Const.UIntPtr _)), _) :: _, Some dflt -> + | TCase((DecisionTreeTest.StringLengthZero _ | DecisionTreeTest.ArrayLength _ | DecisionTreeTest.Const (Const.Single _ | Const.Double _ | Const.String _ | Const.Decimal _ | Const.Int64 _ | Const.UInt64 _ | Const.IntPtr _ | Const.UIntPtr _)), _) :: _, Some dflt -> List.foldBack (fun (TCase(discrim, tree)) sofar -> let testexpr = expr let testexpr = match discrim with + | DecisionTreeTest.StringLengthZero _ -> + let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr + mkLetBind m bind (mkLazyAnd g m (mkNonNullTest g m vExpr) (mkILAsmCeq g m (mkGetStringLength g m vExpr) (mkInt g m 0))) | DecisionTreeTest.ArrayLength(n, _) -> let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr mkLetBind m bind (mkLazyAnd g m (mkNonNullTest g m vExpr) (mkILAsmCeq g m (mkLdlen g m vExpr) (mkInt g m n))) @@ -1547,6 +1557,9 @@ let CompilePatternBasic match discrim with | DecisionTreeTest.Const c2 when (c1=c2) -> [Frontier (i, newActives, valMap)] + | DecisionTreeTest.StringLengthZero _ when (c1 = Const.String "") -> + // Empty string constant matches StringLengthZero discriminator + [Frontier (i, newActives, valMap)] | DecisionTreeTest.Const _ -> // All constants refute all other constants (no overlapping between constants!) [] diff --git a/src/Compiler/Checking/PostInferenceChecks.fs b/src/Compiler/Checking/PostInferenceChecks.fs index b9a3b4df351..45b271973f9 100644 --- a/src/Compiler/Checking/PostInferenceChecks.fs +++ b/src/Compiler/Checking/PostInferenceChecks.fs @@ -1918,6 +1918,7 @@ and CheckDecisionTreeTest cenv env m discrim = | DecisionTreeTest.ArrayLength (_, ty) -> CheckTypeNoInnerByrefs cenv env m ty | DecisionTreeTest.Const _ -> () | DecisionTreeTest.IsNull -> () + | DecisionTreeTest.StringLengthZero ty -> CheckTypeNoInnerByrefs cenv env m ty | DecisionTreeTest.IsInst (srcTy, tgtTy) -> CheckTypeNoInnerByrefs cenv env m srcTy; CheckTypeNoInnerByrefs cenv env m tgtTy | DecisionTreeTest.ActivePatternCase (exp, _, _, _, _, _) -> CheckExprNoByrefs cenv env exp | DecisionTreeTest.Error _ -> () diff --git a/src/Compiler/Checking/QuotationTranslator.fs b/src/Compiler/Checking/QuotationTranslator.fs index ed37c3ba46f..93a0587b5c8 100644 --- a/src/Compiler/Checking/QuotationTranslator.fs +++ b/src/Compiler/Checking/QuotationTranslator.fs @@ -1111,6 +1111,15 @@ and ConvDecisionTree cenv env tgs typR x = let eqR = ConvExpr cenv env eq QP.mkCond (eqR, ConvDecisionTree cenv env tgs typR dtree, acc) + | DecisionTreeTest.StringLengthZero _ -> + // Convert string length zero test to a condition: s != null && s.Length = 0 + let lengthExpr = mkGetStringLength cenv.g m e1 + let lengthTest = mkILAsmCeq cenv.g m lengthExpr (mkInt cenv.g m 0) + let nullTest = mkNonNullTest cenv.g m e1 + let combinedTest = mkLazyAnd cenv.g m nullTest lengthTest + let combinedTestR = ConvExpr cenv env combinedTest + QP.mkCond (combinedTestR, ConvDecisionTree cenv env tgs typR dtree, acc) + | DecisionTreeTest.IsInst (_srcTy, tgtTy) -> let e1R = ConvExpr cenv env e1 QP.mkCond (mkTypeTest (ConvType cenv env m tgtTy, e1R), ConvDecisionTree cenv env tgs typR dtree, acc) diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs index b4460b92e55..22a6b89a55c 100644 --- a/src/Compiler/CodeGen/IlxGen.fs +++ b/src/Compiler/CodeGen/IlxGen.fs @@ -4831,6 +4831,7 @@ and eligibleForFilter (cenv: cenv) expr = | DecisionTreeTest.ArrayLength _ -> true | DecisionTreeTest.Const _ -> true | DecisionTreeTest.IsNull -> true + | DecisionTreeTest.StringLengthZero _ -> true | DecisionTreeTest.IsInst _ -> true | DecisionTreeTest.ActivePatternCase _ -> false // must only be run once | DecisionTreeTest.Error _ -> false @@ -7783,6 +7784,7 @@ and GenDecisionTreeSwitch | DecisionTreeTest.ArrayLength _ | DecisionTreeTest.IsInst _ | DecisionTreeTest.IsNull + | DecisionTreeTest.StringLengthZero _ | DecisionTreeTest.Const(Const.Zero) -> if not (isSingleton cases) || Option.isNone defaultTargetOpt then failwith "internal error: GenDecisionTreeSwitch: DecisionTreeTest.IsInst/isnull/query" diff --git a/src/Compiler/Symbols/Exprs.fs b/src/Compiler/Symbols/Exprs.fs index 3a514db3df7..3a668850c97 100644 --- a/src/Compiler/Symbols/Exprs.fs +++ b/src/Compiler/Symbols/Exprs.fs @@ -1359,6 +1359,14 @@ module FSharpExprConvert = let env = { env with suppressWitnesses = true } ConvExpr cenv env eq E.IfThenElse (eqR, ConvDecisionTree cenv env dtreeRetTy dtree m, acc) + | DecisionTreeTest.StringLengthZero _ -> + // Convert string length zero test to a condition + let lengthExpr = mkGetStringLength g m inpExpr + let lengthTest = mkILAsmCeq g m lengthExpr (mkInt g m 0) + let nullTest = mkNonNullTest g m inpExpr + let combinedTest = mkLazyAnd g m nullTest lengthTest + let combinedTestR = ConvExpr cenv env combinedTest + E.IfThenElse (combinedTestR, ConvDecisionTree cenv env dtreeRetTy dtree m, acc) | DecisionTreeTest.IsInst (_srcTy, tgtTy) -> let e1R = ConvExpr cenv env inpExpr E.IfThenElse (E.TypeTest (ConvType cenv tgtTy, e1R) |> Mk cenv m g.bool_ty, ConvDecisionTree cenv env dtreeRetTy dtree m, acc) diff --git a/src/Compiler/TypedTree/TypedTree.fs b/src/Compiler/TypedTree/TypedTree.fs index e7be325ce33..7e21bf07465 100644 --- a/src/Compiler/TypedTree/TypedTree.fs +++ b/src/Compiler/TypedTree/TypedTree.fs @@ -4803,6 +4803,9 @@ type DecisionTreeTest = /// Test if the input to a decision tree is null | IsNull + /// Test if the input to a decision tree is a string with length zero + | StringLengthZero of ty: TType + /// IsInst(source, target) /// /// Test if the input to a decision tree is an instance of the given type diff --git a/src/Compiler/TypedTree/TypedTree.fsi b/src/Compiler/TypedTree/TypedTree.fsi index 20014a13a64..ad59624d31d 100644 --- a/src/Compiler/TypedTree/TypedTree.fsi +++ b/src/Compiler/TypedTree/TypedTree.fsi @@ -3391,6 +3391,9 @@ type DecisionTreeTest = /// Test if the input to a decision tree is null | IsNull + /// Test if the input to a decision tree is a string with length zero + | StringLengthZero of ty: TType + /// IsInst(source, target) /// /// Test if the input to a decision tree is an instance of the given type diff --git a/src/Compiler/TypedTree/TypedTreeOps.fs b/src/Compiler/TypedTree/TypedTreeOps.fs index 75fd61a7382..0546fced154 100644 --- a/src/Compiler/TypedTree/TypedTreeOps.fs +++ b/src/Compiler/TypedTree/TypedTreeOps.fs @@ -4730,6 +4730,7 @@ module DebugPrint = | DecisionTreeTest.ArrayLength (n, ty) -> wordL(tagText "length") ^^ intL n ^^ typeL ty | DecisionTreeTest.Const c -> wordL(tagText "is") ^^ constL c | DecisionTreeTest.IsNull -> wordL(tagText "isnull") + | DecisionTreeTest.StringLengthZero _ -> wordL(tagText "is") ^^ wordL(tagText "empty string") | DecisionTreeTest.IsInst (_, ty) -> wordL(tagText "isinst") ^^ typeL ty | DecisionTreeTest.ActivePatternCase (exp, _, _, _, _, _) -> wordL(tagText "query") ^^ exprL exp | DecisionTreeTest.Error _ -> wordL (tagText "error recovery") @@ -5353,6 +5354,7 @@ and accFreeInTest (opts: FreeVarOptions) discrim acc = | DecisionTreeTest.ArrayLength(_, ty) -> accFreeVarsInTy opts ty acc | DecisionTreeTest.Const _ | DecisionTreeTest.IsNull -> acc + | DecisionTreeTest.StringLengthZero ty -> accFreeVarsInTy opts ty acc | DecisionTreeTest.IsInst (srcTy, tgtTy) -> accFreeVarsInTy opts srcTy (accFreeVarsInTy opts tgtTy acc) | DecisionTreeTest.ActivePatternCase (exp, tys, _, activePatIdentity, _, _) -> accFreeInExpr opts exp @@ -6226,6 +6228,7 @@ and remapDecisionTree ctxt compgen tmenv x = | DecisionTreeTest.Const _ -> test | DecisionTreeTest.IsInst (srcTy, tgtTy) -> DecisionTreeTest.IsInst (remapType tmenv srcTy, remapType tmenv tgtTy) | DecisionTreeTest.IsNull -> DecisionTreeTest.IsNull + | DecisionTreeTest.StringLengthZero ty -> DecisionTreeTest.StringLengthZero (remapType tmenv ty) | DecisionTreeTest.ActivePatternCase _ -> failwith "DecisionTreeTest.ActivePatternCase should only be used during pattern match compilation" | DecisionTreeTest.Error(m) -> DecisionTreeTest.Error(m) let subTreeR = remapDecisionTree ctxt compgen tmenv subTree diff --git a/src/Compiler/TypedTree/TypedTreeOps.fsi b/src/Compiler/TypedTree/TypedTreeOps.fsi index a68ffaf8af1..78f476f971f 100755 --- a/src/Compiler/TypedTree/TypedTreeOps.fsi +++ b/src/Compiler/TypedTree/TypedTreeOps.fsi @@ -2348,6 +2348,8 @@ val mkIncr: TcGlobals -> range -> Expr -> Expr val mkLdlen: TcGlobals -> range -> Expr -> Expr +val mkGetStringLength: TcGlobals -> range -> Expr -> Expr + val mkLdelem: TcGlobals -> range -> TType -> Expr -> Expr -> Expr //------------------------------------------------------------------------- diff --git a/src/Compiler/TypedTree/TypedTreePickle.fs b/src/Compiler/TypedTree/TypedTreePickle.fs index 8a61809ab06..9e9a01f9fd6 100644 --- a/src/Compiler/TypedTree/TypedTreePickle.fs +++ b/src/Compiler/TypedTree/TypedTreePickle.fs @@ -3442,6 +3442,9 @@ and p_dtree_discrim x st = | DecisionTreeTest.ArrayLength(n, ty) -> p_byte 4 st p_tup2 p_int p_ty (n, ty) st + | DecisionTreeTest.StringLengthZero ty -> + p_byte 5 st + p_ty ty st | DecisionTreeTest.ActivePatternCase _ -> pfailwith st "DecisionTreeTest.ActivePatternCase: only used during pattern match compilation" | DecisionTreeTest.Error _ -> pfailwith st "DecisionTreeTest.Error: only used during pattern match compilation" @@ -3488,6 +3491,7 @@ and u_dtree_discrim st = | 2 -> DecisionTreeTest.IsNull | 3 -> u_tup2 u_ty u_ty st |> DecisionTreeTest.IsInst | 4 -> u_tup2 u_int u_ty st |> DecisionTreeTest.ArrayLength + | 5 -> u_ty st |> DecisionTreeTest.StringLengthZero | _ -> ufailwith st "u_dtree_discrim" and u_target st = From 2e05600d1d9e24c9f964427c27e2856a3d0f305d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 16:14:20 +0000 Subject: [PATCH 03/12] Add IL test for empty string pattern optimization Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- .../EmittedIL/TypeTestsInPatternMatching.fs | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/TypeTestsInPatternMatching.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/TypeTestsInPatternMatching.fs index 7ebd99b4671..8dbdf763f4d 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/TypeTestsInPatternMatching.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/TypeTestsInPatternMatching.fs @@ -314,4 +314,37 @@ let TestTwoColumnsOfTypeTestsWithSealedTypes(x: obj, y: obj) = IL_00d9: ret } """ - ] \ No newline at end of file + ] + + [] + let ``Test codegen for empty string pattern with null safety``() = + FSharp """ +module Test +let TestEmptyStringPattern(x: string) = + match x with + | "" -> "empty" + | _ -> "other" + """ + |> compile + |> shouldSucceed + |> verifyIL [ + """ + .method public static string TestEmptyStringPattern(string x) cil managed + { + + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: brfalse.s IL_0011 + + IL_0003: ldarg.0 + IL_0004: callvirt instance int32 [runtime]System.String::get_Length() + IL_0009: brtrue.s IL_0011 + + IL_000b: ldstr "empty" + IL_0010: ret + + IL_0011: ldstr "other" + IL_0016: ret + } +""" + ] \ No newline at end of file From 59554999019975d4e443a9c77759e66f7203e1e1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 16:20:29 +0000 Subject: [PATCH 04/12] Add release notes for empty string pattern optimization Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- docs/release-notes/.FSharp.Compiler.Service/11.0.0.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.0.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.0.md index 5f2d1149fed..096616b9a4a 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.0.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.0.md @@ -14,6 +14,7 @@ * Type relations cache: handle unsolved type variables ([Issue #19037](https://github.com/dotnet/fsharp/issues/19037)) ([PR #19040](https://github.com/dotnet/fsharp/pull/19040)) * Fix insertion context for modules with multiline attributes. ([Issue #18671](https://github.com/dotnet/fsharp/issues/18671)) * Fix `--typecheck-only` for scripts stopping after processing `#load`-ed script ([PR #19048](https://github.com/dotnet/fsharp/pull/19048)) +* Optimize empty string pattern matching to use null-safe .Length check instead of string equality comparison for better performance. ### Added From 06fcf40fcf442059b11ca37e027255ef861e4f8a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 6 Jan 2026 10:44:21 +0000 Subject: [PATCH 05/12] Fix regression: revert StringLengthZero discriminator, keep optimization in BuildSwitch only Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- .../Checking/PatternMatchCompilation.fs | 19 +++++-------------- src/Compiler/Checking/QuotationTranslator.fs | 9 ++------- src/Compiler/CodeGen/IlxGen.fs | 4 ++-- src/Compiler/Symbols/Exprs.fs | 9 ++------- src/Compiler/TypedTree/TypedTreePickle.fs | 8 ++++---- 5 files changed, 15 insertions(+), 34 deletions(-) diff --git a/src/Compiler/Checking/PatternMatchCompilation.fs b/src/Compiler/Checking/PatternMatchCompilation.fs index f816ffa3f7b..4de6092439c 100644 --- a/src/Compiler/Checking/PatternMatchCompilation.fs +++ b/src/Compiler/Checking/PatternMatchCompilation.fs @@ -611,8 +611,6 @@ let getDiscrimOfPattern (g: TcGlobals) tpinst t = Some(DecisionTreeTest.IsInst (instType tpinst srcTy, instType tpinst tgtTy)) | TPat_exnconstr(tcref, _, _m) -> Some(DecisionTreeTest.IsInst (g.exn_ty, mkWoNullAppTy tcref [])) - | TPat_const (Const.String "", _m) -> - Some(DecisionTreeTest.StringLengthZero g.string_ty) | TPat_const (c, _m) -> Some(DecisionTreeTest.Const c) | TPat_unioncase (c, tyargs', _, _m) -> @@ -641,7 +639,6 @@ let discrimsEq (g: TcGlobals) d1 d2 = | DecisionTreeTest.ArrayLength (n1, _), DecisionTreeTest.ArrayLength(n2, _) -> (n1=n2) | DecisionTreeTest.Const c1, DecisionTreeTest.Const c2 -> (c1=c2) | DecisionTreeTest.IsNull, DecisionTreeTest.IsNull -> true - | DecisionTreeTest.StringLengthZero _, DecisionTreeTest.StringLengthZero _ -> true | DecisionTreeTest.IsInst (srcTy1, tgtTy1), DecisionTreeTest.IsInst (srcTy2, tgtTy2) -> typeEquiv g srcTy1 srcTy2 && typeEquiv g tgtTy1 tgtTy2 | DecisionTreeTest.ActivePatternCase (_, _, _, vrefOpt1, n1, _), DecisionTreeTest.ActivePatternCase (_, _, _, vrefOpt2, n2, _) -> match vrefOpt1, vrefOpt2 with @@ -661,9 +658,6 @@ let isDiscrimSubsumedBy g amap m discrim taken = computeWhatFailingNullTestImpliesAboutTypeTest g tgtTy2 = Implication.Fails | DecisionTreeTest.IsInst (_, tgtTy1), DecisionTreeTest.IsNull -> computeWhatFailingTypeTestImpliesAboutNullTest g tgtTy1 = Implication.Fails - | DecisionTreeTest.IsNull, DecisionTreeTest.StringLengthZero _ -> - // Null subsumes empty string - if null is already handled, we don't need a separate empty string case - true | _ -> false @@ -695,7 +689,6 @@ let discrimWithinSimultaneousClass g amap m discrim prev = match discrim, prev with | _, [] -> true | DecisionTreeTest.Const _, DecisionTreeTest.Const _ :: _ - | DecisionTreeTest.StringLengthZero _, DecisionTreeTest.StringLengthZero _ :: _ | DecisionTreeTest.ArrayLength _, DecisionTreeTest.ArrayLength _ :: _ | DecisionTreeTest.UnionCase _, DecisionTreeTest.UnionCase _ :: _ -> true @@ -798,18 +791,19 @@ let rec BuildSwitch inpExprOpt g expr edges dflt m = error(InternalError("inexhaustive match - need a default case!", m)) // Split string, float, uint64, int64, unativeint, nativeint matches into serial equality tests - | TCase((DecisionTreeTest.StringLengthZero _ | DecisionTreeTest.ArrayLength _ | DecisionTreeTest.Const (Const.Single _ | Const.Double _ | Const.String _ | Const.Decimal _ | Const.Int64 _ | Const.UInt64 _ | Const.IntPtr _ | Const.UIntPtr _)), _) :: _, Some dflt -> + | TCase((DecisionTreeTest.ArrayLength _ | DecisionTreeTest.Const (Const.Single _ | Const.Double _ | Const.String _ | Const.Decimal _ | Const.Int64 _ | Const.UInt64 _ | Const.IntPtr _ | Const.UIntPtr _)), _) :: _, Some dflt -> List.foldBack (fun (TCase(discrim, tree)) sofar -> let testexpr = expr let testexpr = match discrim with - | DecisionTreeTest.StringLengthZero _ -> - let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr - mkLetBind m bind (mkLazyAnd g m (mkNonNullTest g m vExpr) (mkILAsmCeq g m (mkGetStringLength g m vExpr) (mkInt g m 0))) | DecisionTreeTest.ArrayLength(n, _) -> let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr mkLetBind m bind (mkLazyAnd g m (mkNonNullTest g m vExpr) (mkILAsmCeq g m (mkLdlen g m vExpr) (mkInt g m n))) + | DecisionTreeTest.Const (Const.String "") -> + // Optimize empty string check to use null-safe length check + let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr + mkLetBind m bind (mkLazyAnd g m (mkNonNullTest g m vExpr) (mkILAsmCeq g m (mkGetStringLength g m vExpr) (mkInt g m 0))) | DecisionTreeTest.Const (Const.String _ as c) -> mkCallEqualsOperator g m g.string_ty testexpr (Expr.Const (c, m, g.string_ty)) | DecisionTreeTest.Const (Const.Decimal _ as c) -> @@ -1557,9 +1551,6 @@ let CompilePatternBasic match discrim with | DecisionTreeTest.Const c2 when (c1=c2) -> [Frontier (i, newActives, valMap)] - | DecisionTreeTest.StringLengthZero _ when (c1 = Const.String "") -> - // Empty string constant matches StringLengthZero discriminator - [Frontier (i, newActives, valMap)] | DecisionTreeTest.Const _ -> // All constants refute all other constants (no overlapping between constants!) [] diff --git a/src/Compiler/Checking/QuotationTranslator.fs b/src/Compiler/Checking/QuotationTranslator.fs index 93a0587b5c8..ca84e69e974 100644 --- a/src/Compiler/Checking/QuotationTranslator.fs +++ b/src/Compiler/Checking/QuotationTranslator.fs @@ -1112,13 +1112,8 @@ and ConvDecisionTree cenv env tgs typR x = QP.mkCond (eqR, ConvDecisionTree cenv env tgs typR dtree, acc) | DecisionTreeTest.StringLengthZero _ -> - // Convert string length zero test to a condition: s != null && s.Length = 0 - let lengthExpr = mkGetStringLength cenv.g m e1 - let lengthTest = mkILAsmCeq cenv.g m lengthExpr (mkInt cenv.g m 0) - let nullTest = mkNonNullTest cenv.g m e1 - let combinedTest = mkLazyAnd cenv.g m nullTest lengthTest - let combinedTestR = ConvExpr cenv env combinedTest - QP.mkCond (combinedTestR, ConvDecisionTree cenv env tgs typR dtree, acc) + // StringLengthZero should not be used in pattern compilation; empty strings use Const + wfail(InternalError( "DecisionTreeTest.StringLengthZero in quoted expression", m)) | DecisionTreeTest.IsInst (_srcTy, tgtTy) -> let e1R = ConvExpr cenv env e1 diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs index 22a6b89a55c..0ad851684bb 100644 --- a/src/Compiler/CodeGen/IlxGen.fs +++ b/src/Compiler/CodeGen/IlxGen.fs @@ -4831,7 +4831,7 @@ and eligibleForFilter (cenv: cenv) expr = | DecisionTreeTest.ArrayLength _ -> true | DecisionTreeTest.Const _ -> true | DecisionTreeTest.IsNull -> true - | DecisionTreeTest.StringLengthZero _ -> true + | DecisionTreeTest.StringLengthZero _ -> true // Should not be emitted but keep for safety | DecisionTreeTest.IsInst _ -> true | DecisionTreeTest.ActivePatternCase _ -> false // must only be run once | DecisionTreeTest.Error _ -> false @@ -7784,7 +7784,7 @@ and GenDecisionTreeSwitch | DecisionTreeTest.ArrayLength _ | DecisionTreeTest.IsInst _ | DecisionTreeTest.IsNull - | DecisionTreeTest.StringLengthZero _ + | DecisionTreeTest.StringLengthZero _ // Should not be emitted but keep for safety | DecisionTreeTest.Const(Const.Zero) -> if not (isSingleton cases) || Option.isNone defaultTargetOpt then failwith "internal error: GenDecisionTreeSwitch: DecisionTreeTest.IsInst/isnull/query" diff --git a/src/Compiler/Symbols/Exprs.fs b/src/Compiler/Symbols/Exprs.fs index 3a668850c97..8668f565259 100644 --- a/src/Compiler/Symbols/Exprs.fs +++ b/src/Compiler/Symbols/Exprs.fs @@ -1360,13 +1360,8 @@ module FSharpExprConvert = ConvExpr cenv env eq E.IfThenElse (eqR, ConvDecisionTree cenv env dtreeRetTy dtree m, acc) | DecisionTreeTest.StringLengthZero _ -> - // Convert string length zero test to a condition - let lengthExpr = mkGetStringLength g m inpExpr - let lengthTest = mkILAsmCeq g m lengthExpr (mkInt g m 0) - let nullTest = mkNonNullTest g m inpExpr - let combinedTest = mkLazyAnd g m nullTest lengthTest - let combinedTestR = ConvExpr cenv env combinedTest - E.IfThenElse (combinedTestR, ConvDecisionTree cenv env dtreeRetTy dtree m, acc) + // StringLengthZero should not be used in pattern compilation; empty strings use Const + wfail("StringLengthZero discriminator should not be used", m) | DecisionTreeTest.IsInst (_srcTy, tgtTy) -> let e1R = ConvExpr cenv env inpExpr E.IfThenElse (E.TypeTest (ConvType cenv tgtTy, e1R) |> Mk cenv m g.bool_ty, ConvDecisionTree cenv env dtreeRetTy dtree m, acc) diff --git a/src/Compiler/TypedTree/TypedTreePickle.fs b/src/Compiler/TypedTree/TypedTreePickle.fs index 9e9a01f9fd6..8675e84e2bd 100644 --- a/src/Compiler/TypedTree/TypedTreePickle.fs +++ b/src/Compiler/TypedTree/TypedTreePickle.fs @@ -3442,9 +3442,10 @@ and p_dtree_discrim x st = | DecisionTreeTest.ArrayLength(n, ty) -> p_byte 4 st p_tup2 p_int p_ty (n, ty) st - | DecisionTreeTest.StringLengthZero ty -> - p_byte 5 st - p_ty ty st + | DecisionTreeTest.StringLengthZero _ -> + // Pickle as Const "" for backward compatibility with older compilers + p_byte 1 st + p_const (Const.String "") st | DecisionTreeTest.ActivePatternCase _ -> pfailwith st "DecisionTreeTest.ActivePatternCase: only used during pattern match compilation" | DecisionTreeTest.Error _ -> pfailwith st "DecisionTreeTest.Error: only used during pattern match compilation" @@ -3491,7 +3492,6 @@ and u_dtree_discrim st = | 2 -> DecisionTreeTest.IsNull | 3 -> u_tup2 u_ty u_ty st |> DecisionTreeTest.IsInst | 4 -> u_tup2 u_int u_ty st |> DecisionTreeTest.ArrayLength - | 5 -> u_ty st |> DecisionTreeTest.StringLengthZero | _ -> ufailwith st "u_dtree_discrim" and u_target st = From 60a961400f9905be7ed179642140b5a38328c8ad Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 6 Jan 2026 11:03:10 +0000 Subject: [PATCH 06/12] Add inlining test for empty string pattern optimization with pickle compatibility Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- .../EmittedIL/Inlining/EmptyStringPattern.fs | 12 ++ .../Inlining/EmptyStringPattern.fs.il.bsl | 107 ++++++++++++++++++ .../EmittedIL/Inlining/Inlining.fs | 7 ++ 3 files changed, 126 insertions(+) create mode 100644 tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs create mode 100644 tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs.il.bsl diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs new file mode 100644 index 00000000000..5d1af068e8e --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs @@ -0,0 +1,12 @@ +module TestLibrary + +let inline classifyString (s: string) = + match s with + | "" -> "empty" + | null -> "null" + | _ -> "other" + +let inline testEmptyStringOnly (s: string) = + match s with + | "" -> 1 + | _ -> 0 diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs.il.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs.il.bsl new file mode 100644 index 00000000000..57a1fb8c900 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs.il.bsl @@ -0,0 +1,107 @@ + + + + + +.assembly extern runtime { } +.assembly extern FSharp.Core { } +.assembly assembly +{ + .custom instance void [FSharp.Core]Microsoft.FSharp.Core.FSharpInterfaceDataVersionAttribute::.ctor(int32, + int32, + int32) = ( 01 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 ) + + + + + .hash algorithm 0x00008004 + .ver 0:0:0:0 +} +.module assembly.exe + +.imagebase {value} +.file alignment 0x00000200 +.stackreserve 0x00100000 +.subsystem 0x0003 +.corflags 0x00000001 + + + + + +.class public abstract auto ansi sealed TestLibrary + extends [runtime]System.Object +{ + .custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags) = ( 01 00 07 00 00 00 00 00 ) + .method public static string classifyString(string s) cil managed + { + + .maxstack 8 + IL_0000: nop + IL_0001: ldarg.0 + IL_0002: brfalse.s IL_0010 + + IL_0004: ldarg.0 + IL_0005: callvirt instance int32 [runtime]System.String::get_Length() + IL_000a: ldc.i4.0 + IL_000b: ceq + IL_000d: nop + IL_000e: br.s IL_0012 + + IL_0010: ldc.i4.0 + IL_0011: nop + IL_0012: brtrue.s IL_0019 + + IL_0014: ldarg.0 + IL_0015: brfalse.s IL_001f + + IL_0017: br.s IL_0025 + + IL_0019: ldstr "empty" + IL_001e: ret + + IL_001f: ldstr "null" + IL_0024: ret + + IL_0025: ldstr "other" + IL_002a: ret + } + + .method public static int32 testEmptyStringOnly(string s) cil managed + { + + .maxstack 8 + IL_0000: nop + IL_0001: ldarg.0 + IL_0002: brfalse.s IL_000e + + IL_0004: ldarg.0 + IL_0005: callvirt instance int32 [runtime]System.String::get_Length() + IL_000a: brtrue.s IL_000e + + IL_000c: ldc.i4.1 + IL_000d: ret + + IL_000e: ldc.i4.0 + IL_000f: ret + } + +} + +.class private abstract auto ansi sealed ''.$TestLibrary + extends [runtime]System.Object +{ + .method public static void main@() cil managed + { + .entrypoint + + .maxstack 8 + IL_0000: ret + } + +} + + + + + diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/Inlining.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/Inlining.fs index ffd75e0d081..b34c97af3ec 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/Inlining.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/Inlining.fs @@ -229,3 +229,10 @@ printfn $"{(SecondType ()).SecondMethod()}" |> compileAndRun |> shouldSucceed + + // Test empty string pattern optimization in inlining with pickle compatibility + [] + let ``EmptyStringPattern_fs`` compilation = + compilation + |> getCompilation + |> verifyCompilation From 64ae956295c406ebda1027cc890a9757b1c7a0a3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 6 Jan 2026 14:55:01 +0000 Subject: [PATCH 07/12] Remove all StringLengthZero usages, update tests with bundled clauses, move release notes to 10.0.200 Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- .../.FSharp.Compiler.Service/10.0.200.md | 1 + .../.FSharp.Compiler.Service/11.0.0.md | 1 - src/Compiler/Checking/FindUnsolved.fs | 1 - src/Compiler/Checking/PostInferenceChecks.fs | 1 - src/Compiler/Checking/QuotationTranslator.fs | 4 - src/Compiler/CodeGen/IlxGen.fs | 2 - src/Compiler/Symbols/Exprs.fs | 3 - src/Compiler/TypedTree/TypedTree.fs | 3 - src/Compiler/TypedTree/TypedTree.fsi | 3 - src/Compiler/TypedTree/TypedTreeOps.fs | 3 - src/Compiler/TypedTree/TypedTreePickle.fs | 4 - .../EmittedIL/Inlining/EmptyStringPattern.fs | 16 ++ .../Inlining/EmptyStringPattern.fs.il.bsl | 169 ++++++++++++++++++ .../EmittedIL/Inlining/Inlining.fs | 2 +- 14 files changed, 187 insertions(+), 26 deletions(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/10.0.200.md b/docs/release-notes/.FSharp.Compiler.Service/10.0.200.md index 9312da9dda8..2c85c7f4022 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/10.0.200.md +++ b/docs/release-notes/.FSharp.Compiler.Service/10.0.200.md @@ -3,6 +3,7 @@ * Type relations cache: optimize key generation ([Issue #19116](https://github.com/dotnet/fsharp/issues/18767)) ([PR #19120](https://github.com/dotnet/fsharp/pull/19120)) * Fixed QuickParse to correctly handle optional parameter syntax with `?` prefix, resolving syntax highlighting issues. ([Issue #11008753](https://developercommunity.visualstudio.com/t/F-Highlighting-fails-on-optional-parame/11008753)) ([PR #XXXXX](https://github.com/dotnet/fsharp/pull/XXXXX)) * Fix `--preferreduilang` switch leaking into `fsi.CommandLineArgs` when positioned after script file ([PR #19151](https://github.com/dotnet/fsharp/pull/19151)) +* Optimize empty string pattern matching to use null-safe .Length check instead of string equality comparison for better performance. ### Added diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.0.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.0.md index 096616b9a4a..5f2d1149fed 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.0.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.0.md @@ -14,7 +14,6 @@ * Type relations cache: handle unsolved type variables ([Issue #19037](https://github.com/dotnet/fsharp/issues/19037)) ([PR #19040](https://github.com/dotnet/fsharp/pull/19040)) * Fix insertion context for modules with multiline attributes. ([Issue #18671](https://github.com/dotnet/fsharp/issues/18671)) * Fix `--typecheck-only` for scripts stopping after processing `#load`-ed script ([PR #19048](https://github.com/dotnet/fsharp/pull/19048)) -* Optimize empty string pattern matching to use null-safe .Length check instead of string equality comparison for better performance. ### Added diff --git a/src/Compiler/Checking/FindUnsolved.fs b/src/Compiler/Checking/FindUnsolved.fs index d3424a4bf6b..c1de629a07c 100644 --- a/src/Compiler/Checking/FindUnsolved.fs +++ b/src/Compiler/Checking/FindUnsolved.fs @@ -216,7 +216,6 @@ and accDiscrim cenv env d mFallback = | DecisionTreeTest.ArrayLength(_, ty) -> accTy cenv env mFallback ty | DecisionTreeTest.Const _ | DecisionTreeTest.IsNull -> () - | DecisionTreeTest.StringLengthZero ty -> accTy cenv env mFallback ty | DecisionTreeTest.IsInst (srcTy, tgtTy) -> accTy cenv env mFallback srcTy; accTy cenv env mFallback tgtTy | DecisionTreeTest.ActivePatternCase (exp, tys, _, _, _, _) -> accExpr cenv env exp diff --git a/src/Compiler/Checking/PostInferenceChecks.fs b/src/Compiler/Checking/PostInferenceChecks.fs index 45b271973f9..b9a3b4df351 100644 --- a/src/Compiler/Checking/PostInferenceChecks.fs +++ b/src/Compiler/Checking/PostInferenceChecks.fs @@ -1918,7 +1918,6 @@ and CheckDecisionTreeTest cenv env m discrim = | DecisionTreeTest.ArrayLength (_, ty) -> CheckTypeNoInnerByrefs cenv env m ty | DecisionTreeTest.Const _ -> () | DecisionTreeTest.IsNull -> () - | DecisionTreeTest.StringLengthZero ty -> CheckTypeNoInnerByrefs cenv env m ty | DecisionTreeTest.IsInst (srcTy, tgtTy) -> CheckTypeNoInnerByrefs cenv env m srcTy; CheckTypeNoInnerByrefs cenv env m tgtTy | DecisionTreeTest.ActivePatternCase (exp, _, _, _, _, _) -> CheckExprNoByrefs cenv env exp | DecisionTreeTest.Error _ -> () diff --git a/src/Compiler/Checking/QuotationTranslator.fs b/src/Compiler/Checking/QuotationTranslator.fs index ca84e69e974..ed37c3ba46f 100644 --- a/src/Compiler/Checking/QuotationTranslator.fs +++ b/src/Compiler/Checking/QuotationTranslator.fs @@ -1111,10 +1111,6 @@ and ConvDecisionTree cenv env tgs typR x = let eqR = ConvExpr cenv env eq QP.mkCond (eqR, ConvDecisionTree cenv env tgs typR dtree, acc) - | DecisionTreeTest.StringLengthZero _ -> - // StringLengthZero should not be used in pattern compilation; empty strings use Const - wfail(InternalError( "DecisionTreeTest.StringLengthZero in quoted expression", m)) - | DecisionTreeTest.IsInst (_srcTy, tgtTy) -> let e1R = ConvExpr cenv env e1 QP.mkCond (mkTypeTest (ConvType cenv env m tgtTy, e1R), ConvDecisionTree cenv env tgs typR dtree, acc) diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs index 0ad851684bb..b4460b92e55 100644 --- a/src/Compiler/CodeGen/IlxGen.fs +++ b/src/Compiler/CodeGen/IlxGen.fs @@ -4831,7 +4831,6 @@ and eligibleForFilter (cenv: cenv) expr = | DecisionTreeTest.ArrayLength _ -> true | DecisionTreeTest.Const _ -> true | DecisionTreeTest.IsNull -> true - | DecisionTreeTest.StringLengthZero _ -> true // Should not be emitted but keep for safety | DecisionTreeTest.IsInst _ -> true | DecisionTreeTest.ActivePatternCase _ -> false // must only be run once | DecisionTreeTest.Error _ -> false @@ -7784,7 +7783,6 @@ and GenDecisionTreeSwitch | DecisionTreeTest.ArrayLength _ | DecisionTreeTest.IsInst _ | DecisionTreeTest.IsNull - | DecisionTreeTest.StringLengthZero _ // Should not be emitted but keep for safety | DecisionTreeTest.Const(Const.Zero) -> if not (isSingleton cases) || Option.isNone defaultTargetOpt then failwith "internal error: GenDecisionTreeSwitch: DecisionTreeTest.IsInst/isnull/query" diff --git a/src/Compiler/Symbols/Exprs.fs b/src/Compiler/Symbols/Exprs.fs index 8668f565259..3a514db3df7 100644 --- a/src/Compiler/Symbols/Exprs.fs +++ b/src/Compiler/Symbols/Exprs.fs @@ -1359,9 +1359,6 @@ module FSharpExprConvert = let env = { env with suppressWitnesses = true } ConvExpr cenv env eq E.IfThenElse (eqR, ConvDecisionTree cenv env dtreeRetTy dtree m, acc) - | DecisionTreeTest.StringLengthZero _ -> - // StringLengthZero should not be used in pattern compilation; empty strings use Const - wfail("StringLengthZero discriminator should not be used", m) | DecisionTreeTest.IsInst (_srcTy, tgtTy) -> let e1R = ConvExpr cenv env inpExpr E.IfThenElse (E.TypeTest (ConvType cenv tgtTy, e1R) |> Mk cenv m g.bool_ty, ConvDecisionTree cenv env dtreeRetTy dtree m, acc) diff --git a/src/Compiler/TypedTree/TypedTree.fs b/src/Compiler/TypedTree/TypedTree.fs index 7e21bf07465..e7be325ce33 100644 --- a/src/Compiler/TypedTree/TypedTree.fs +++ b/src/Compiler/TypedTree/TypedTree.fs @@ -4803,9 +4803,6 @@ type DecisionTreeTest = /// Test if the input to a decision tree is null | IsNull - /// Test if the input to a decision tree is a string with length zero - | StringLengthZero of ty: TType - /// IsInst(source, target) /// /// Test if the input to a decision tree is an instance of the given type diff --git a/src/Compiler/TypedTree/TypedTree.fsi b/src/Compiler/TypedTree/TypedTree.fsi index ad59624d31d..20014a13a64 100644 --- a/src/Compiler/TypedTree/TypedTree.fsi +++ b/src/Compiler/TypedTree/TypedTree.fsi @@ -3391,9 +3391,6 @@ type DecisionTreeTest = /// Test if the input to a decision tree is null | IsNull - /// Test if the input to a decision tree is a string with length zero - | StringLengthZero of ty: TType - /// IsInst(source, target) /// /// Test if the input to a decision tree is an instance of the given type diff --git a/src/Compiler/TypedTree/TypedTreeOps.fs b/src/Compiler/TypedTree/TypedTreeOps.fs index 0546fced154..75fd61a7382 100644 --- a/src/Compiler/TypedTree/TypedTreeOps.fs +++ b/src/Compiler/TypedTree/TypedTreeOps.fs @@ -4730,7 +4730,6 @@ module DebugPrint = | DecisionTreeTest.ArrayLength (n, ty) -> wordL(tagText "length") ^^ intL n ^^ typeL ty | DecisionTreeTest.Const c -> wordL(tagText "is") ^^ constL c | DecisionTreeTest.IsNull -> wordL(tagText "isnull") - | DecisionTreeTest.StringLengthZero _ -> wordL(tagText "is") ^^ wordL(tagText "empty string") | DecisionTreeTest.IsInst (_, ty) -> wordL(tagText "isinst") ^^ typeL ty | DecisionTreeTest.ActivePatternCase (exp, _, _, _, _, _) -> wordL(tagText "query") ^^ exprL exp | DecisionTreeTest.Error _ -> wordL (tagText "error recovery") @@ -5354,7 +5353,6 @@ and accFreeInTest (opts: FreeVarOptions) discrim acc = | DecisionTreeTest.ArrayLength(_, ty) -> accFreeVarsInTy opts ty acc | DecisionTreeTest.Const _ | DecisionTreeTest.IsNull -> acc - | DecisionTreeTest.StringLengthZero ty -> accFreeVarsInTy opts ty acc | DecisionTreeTest.IsInst (srcTy, tgtTy) -> accFreeVarsInTy opts srcTy (accFreeVarsInTy opts tgtTy acc) | DecisionTreeTest.ActivePatternCase (exp, tys, _, activePatIdentity, _, _) -> accFreeInExpr opts exp @@ -6228,7 +6226,6 @@ and remapDecisionTree ctxt compgen tmenv x = | DecisionTreeTest.Const _ -> test | DecisionTreeTest.IsInst (srcTy, tgtTy) -> DecisionTreeTest.IsInst (remapType tmenv srcTy, remapType tmenv tgtTy) | DecisionTreeTest.IsNull -> DecisionTreeTest.IsNull - | DecisionTreeTest.StringLengthZero ty -> DecisionTreeTest.StringLengthZero (remapType tmenv ty) | DecisionTreeTest.ActivePatternCase _ -> failwith "DecisionTreeTest.ActivePatternCase should only be used during pattern match compilation" | DecisionTreeTest.Error(m) -> DecisionTreeTest.Error(m) let subTreeR = remapDecisionTree ctxt compgen tmenv subTree diff --git a/src/Compiler/TypedTree/TypedTreePickle.fs b/src/Compiler/TypedTree/TypedTreePickle.fs index 8675e84e2bd..8a61809ab06 100644 --- a/src/Compiler/TypedTree/TypedTreePickle.fs +++ b/src/Compiler/TypedTree/TypedTreePickle.fs @@ -3442,10 +3442,6 @@ and p_dtree_discrim x st = | DecisionTreeTest.ArrayLength(n, ty) -> p_byte 4 st p_tup2 p_int p_ty (n, ty) st - | DecisionTreeTest.StringLengthZero _ -> - // Pickle as Const "" for backward compatibility with older compilers - p_byte 1 st - p_const (Const.String "") st | DecisionTreeTest.ActivePatternCase _ -> pfailwith st "DecisionTreeTest.ActivePatternCase: only used during pattern match compilation" | DecisionTreeTest.Error _ -> pfailwith st "DecisionTreeTest.Error: only used during pattern match compilation" diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs index 5d1af068e8e..ad0c2967690 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs @@ -10,3 +10,19 @@ let inline testEmptyStringOnly (s: string) = match s with | "" -> 1 | _ -> 0 + +let inline testBundledNullAndEmpty (s: string) = + match s with + | null | "" -> 0 + | _ -> 1 + +let inline testBundledEmptyAndNull (s: string) = + match s with + | "" | null -> 0 + | _ -> 1 + +// Usage functions to show inlining in action +let useClassifyString s = classifyString s +let useTestEmptyStringOnly s = testEmptyStringOnly s +let useBundledNullAndEmpty s = testBundledNullAndEmpty s +let useBundledEmptyAndNull s = testBundledEmptyAndNull s diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs.il.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs.il.bsl index 57a1fb8c900..0d1c3c137a5 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs.il.bsl +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs.il.bsl @@ -86,6 +86,175 @@ IL_000f: ret } + .method public static int32 testBundledNullAndEmpty(string s) cil managed + { + + .maxstack 8 + IL_0000: nop + IL_0001: ldarg.0 + IL_0002: brfalse.s IL_0017 + + IL_0004: ldarg.0 + IL_0005: brfalse.s IL_0013 + + IL_0007: ldarg.0 + IL_0008: callvirt instance int32 [runtime]System.String::get_Length() + IL_000d: ldc.i4.0 + IL_000e: ceq + IL_0010: nop + IL_0011: br.s IL_0015 + + IL_0013: ldc.i4.0 + IL_0014: nop + IL_0015: brfalse.s IL_0019 + + IL_0017: ldc.i4.0 + IL_0018: ret + + IL_0019: ldc.i4.1 + IL_001a: ret + } + + .method public static int32 testBundledEmptyAndNull(string s) cil managed + { + + .maxstack 8 + IL_0000: nop + IL_0001: ldarg.0 + IL_0002: brfalse.s IL_0010 + + IL_0004: ldarg.0 + IL_0005: callvirt instance int32 [runtime]System.String::get_Length() + IL_000a: ldc.i4.0 + IL_000b: ceq + IL_000d: nop + IL_000e: br.s IL_0012 + + IL_0010: ldc.i4.0 + IL_0011: nop + IL_0012: brtrue.s IL_0017 + + IL_0014: ldarg.0 + IL_0015: brtrue.s IL_0019 + + IL_0017: ldc.i4.0 + IL_0018: ret + + IL_0019: ldc.i4.1 + IL_001a: ret + } + + .method public static string useClassifyString(string s) cil managed + { + + .maxstack 8 + IL_0000: nop + IL_0001: ldarg.0 + IL_0002: brfalse.s IL_0010 + + IL_0004: ldarg.0 + IL_0005: callvirt instance int32 [runtime]System.String::get_Length() + IL_000a: ldc.i4.0 + IL_000b: ceq + IL_000d: nop + IL_000e: br.s IL_0012 + + IL_0010: ldc.i4.0 + IL_0011: nop + IL_0012: brtrue.s IL_0019 + + IL_0014: ldarg.0 + IL_0015: brfalse.s IL_001f + + IL_0017: br.s IL_0025 + + IL_0019: ldstr "empty" + IL_001e: ret + + IL_001f: ldstr "null" + IL_0024: ret + + IL_0025: ldstr "other" + IL_002a: ret + } + + .method public static int32 useTestEmptyStringOnly(string s) cil managed + { + + .maxstack 8 + IL_0000: nop + IL_0001: ldarg.0 + IL_0002: brfalse.s IL_000e + + IL_0004: ldarg.0 + IL_0005: callvirt instance int32 [runtime]System.String::get_Length() + IL_000a: brtrue.s IL_000e + + IL_000c: ldc.i4.1 + IL_000d: ret + + IL_000e: ldc.i4.0 + IL_000f: ret + } + + .method public static int32 useBundledNullAndEmpty(string s) cil managed + { + + .maxstack 8 + IL_0000: nop + IL_0001: ldarg.0 + IL_0002: brfalse.s IL_0017 + + IL_0004: ldarg.0 + IL_0005: brfalse.s IL_0013 + + IL_0007: ldarg.0 + IL_0008: callvirt instance int32 [runtime]System.String::get_Length() + IL_000d: ldc.i4.0 + IL_000e: ceq + IL_0010: nop + IL_0011: br.s IL_0015 + + IL_0013: ldc.i4.0 + IL_0014: nop + IL_0015: brfalse.s IL_0019 + + IL_0017: ldc.i4.0 + IL_0018: ret + + IL_0019: ldc.i4.1 + IL_001a: ret + } + + .method public static int32 useBundledEmptyAndNull(string s) cil managed + { + + .maxstack 8 + IL_0000: nop + IL_0001: ldarg.0 + IL_0002: brfalse.s IL_0010 + + IL_0004: ldarg.0 + IL_0005: callvirt instance int32 [runtime]System.String::get_Length() + IL_000a: ldc.i4.0 + IL_000b: ceq + IL_000d: nop + IL_000e: br.s IL_0012 + + IL_0010: ldc.i4.0 + IL_0011: nop + IL_0012: brtrue.s IL_0017 + + IL_0014: ldarg.0 + IL_0015: brtrue.s IL_0019 + + IL_0017: ldc.i4.0 + IL_0018: ret + + IL_0019: ldc.i4.1 + IL_001a: ret + } + } .class private abstract auto ansi sealed ''.$TestLibrary diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/Inlining.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/Inlining.fs index b34c97af3ec..0e7e1aa84e3 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/Inlining.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/Inlining.fs @@ -230,7 +230,7 @@ printfn $"{(SecondType ()).SecondMethod()}" |> shouldSucceed - // Test empty string pattern optimization in inlining with pickle compatibility + // Test empty string pattern optimization in inlining [] let ``EmptyStringPattern_fs`` compilation = compilation From c4b35b018e3ada63cc9b4fe47a93daaa2b0a7f1c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 6 Jan 2026 20:35:36 +0000 Subject: [PATCH 08/12] WIP: Add isNullFiltered parameter to BuildSwitch to track null-safe contexts Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- .../Checking/PatternMatchCompilation.fs | 20 +- .../Inlining/EmptyStringPattern.fs.il.bsl | 194 ++++++------------ 2 files changed, 78 insertions(+), 136 deletions(-) diff --git a/src/Compiler/Checking/PatternMatchCompilation.fs b/src/Compiler/Checking/PatternMatchCompilation.fs index 4de6092439c..1370a496f92 100644 --- a/src/Compiler/Checking/PatternMatchCompilation.fs +++ b/src/Compiler/Checking/PatternMatchCompilation.fs @@ -767,7 +767,7 @@ let (|ConstNeedsDefaultCase|_|) c = /// - Compact integer switches become a single switch. Non-compact integer /// switches, string switches and floating point switches are treated in the /// same way as DecisionTreeTest.IsInst. -let rec BuildSwitch inpExprOpt g expr edges dflt m = +let rec BuildSwitch inpExprOpt g isNullFiltered expr edges dflt m = if verbose then dprintf "--> BuildSwitch@%a, #edges = %A, dflt.IsSome = %A\n" outputRange m (List.length edges) (Option.isSome dflt) match edges, dflt with | [], None -> failwith "internal error: no edges and no default" @@ -780,11 +780,13 @@ let rec BuildSwitch inpExprOpt g expr edges dflt m = // In this case the 'expr' already holds the result of the 'isinst' test. | TCase(DecisionTreeTest.IsInst _, success) :: edges, dflt when Option.isSome inpExprOpt -> - TDSwitch(expr, [TCase(DecisionTreeTest.IsNull, BuildSwitch None g expr edges dflt m)], Some success, m) + TDSwitch(expr, [TCase(DecisionTreeTest.IsNull, BuildSwitch None g false expr edges dflt m)], Some success, m) // isnull and isinst tests | TCase((DecisionTreeTest.IsNull | DecisionTreeTest.IsInst _), _) as edge :: edges, dflt -> - TDSwitch(expr, [edge], Some (BuildSwitch None g expr edges dflt m), m) + // After an IsNull test, in the fallthrough branch (Some), we know the value is not null + let nullFiltered = match edge with TCase(DecisionTreeTest.IsNull, _) -> true | _ -> isNullFiltered + TDSwitch(expr, [edge], Some (BuildSwitch None g nullFiltered expr edges dflt m), m) // All these should also always have default cases | TCase(DecisionTreeTest.Const ConstNeedsDefaultCase, _) :: _, None -> @@ -799,11 +801,17 @@ let rec BuildSwitch inpExprOpt g expr edges dflt m = match discrim with | DecisionTreeTest.ArrayLength(n, _) -> let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr - mkLetBind m bind (mkLazyAnd g m (mkNonNullTest g m vExpr) (mkILAsmCeq g m (mkLdlen g m vExpr) (mkInt g m n))) + // Skip null check if we're in a null-filtered context + let test = mkILAsmCeq g m (mkLdlen g m vExpr) (mkInt g m n) + let finalTest = if isNullFiltered then test else mkLazyAnd g m (mkNonNullTest g m vExpr) test + mkLetBind m bind finalTest | DecisionTreeTest.Const (Const.String "") -> // Optimize empty string check to use null-safe length check let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr - mkLetBind m bind (mkLazyAnd g m (mkNonNullTest g m vExpr) (mkILAsmCeq g m (mkGetStringLength g m vExpr) (mkInt g m 0))) + let test = mkILAsmCeq g m (mkGetStringLength g m vExpr) (mkInt g m 0) + // Skip null check if we're in a null-filtered context + let finalTest = if isNullFiltered then test else mkLazyAnd g m (mkNonNullTest g m vExpr) test + mkLetBind m bind finalTest | DecisionTreeTest.Const (Const.String _ as c) -> mkCallEqualsOperator g m g.string_ty testexpr (Expr.Const (c, m, g.string_ty)) | DecisionTreeTest.Const (Const.Decimal _ as c) -> @@ -1156,7 +1164,7 @@ let CompilePatternBasic // OK, build the whole tree and whack on the binding if any let finalDecisionTree = let inpExprToSwitch = (match inpExprOpt with Some vExpr -> vExpr | None -> GetSubExprOfInput subexpr) - let tree = BuildSwitch inpExprOpt g inpExprToSwitch simulSetOfCases defaultTreeOpt mMatch + let tree = BuildSwitch inpExprOpt g false inpExprToSwitch simulSetOfCases defaultTreeOpt mMatch match bindOpt with | None -> tree | Some bind -> TDBind (bind, tree) diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs.il.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs.il.bsl index 0d1c3c137a5..51cf669e07c 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs.il.bsl +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs.il.bsl @@ -39,32 +39,22 @@ .maxstack 8 IL_0000: nop IL_0001: ldarg.0 - IL_0002: brfalse.s IL_0010 + IL_0002: callvirt instance int32 [runtime]System.String::get_Length() + IL_0007: brfalse.s IL_000e - IL_0004: ldarg.0 - IL_0005: callvirt instance int32 [runtime]System.String::get_Length() - IL_000a: ldc.i4.0 - IL_000b: ceq - IL_000d: nop - IL_000e: br.s IL_0012 - - IL_0010: ldc.i4.0 - IL_0011: nop - IL_0012: brtrue.s IL_0019 + IL_0009: ldarg.0 + IL_000a: brfalse.s IL_0014 - IL_0014: ldarg.0 - IL_0015: brfalse.s IL_001f + IL_000c: br.s IL_001a - IL_0017: br.s IL_0025 + IL_000e: ldstr "empty" + IL_0013: ret - IL_0019: ldstr "empty" - IL_001e: ret + IL_0014: ldstr "null" + IL_0019: ret - IL_001f: ldstr "null" - IL_0024: ret - - IL_0025: ldstr "other" - IL_002a: ret + IL_001a: ldstr "other" + IL_001f: ret } .method public static int32 testEmptyStringOnly(string s) cil managed @@ -73,17 +63,14 @@ .maxstack 8 IL_0000: nop IL_0001: ldarg.0 - IL_0002: brfalse.s IL_000e - - IL_0004: ldarg.0 - IL_0005: callvirt instance int32 [runtime]System.String::get_Length() - IL_000a: brtrue.s IL_000e + IL_0002: callvirt instance int32 [runtime]System.String::get_Length() + IL_0007: brtrue.s IL_000b - IL_000c: ldc.i4.1 - IL_000d: ret + IL_0009: ldc.i4.1 + IL_000a: ret - IL_000e: ldc.i4.0 - IL_000f: ret + IL_000b: ldc.i4.0 + IL_000c: ret } .method public static int32 testBundledNullAndEmpty(string s) cil managed @@ -92,27 +79,17 @@ .maxstack 8 IL_0000: nop IL_0001: ldarg.0 - IL_0002: brfalse.s IL_0017 + IL_0002: brfalse.s IL_000c IL_0004: ldarg.0 - IL_0005: brfalse.s IL_0013 - - IL_0007: ldarg.0 - IL_0008: callvirt instance int32 [runtime]System.String::get_Length() - IL_000d: ldc.i4.0 - IL_000e: ceq - IL_0010: nop - IL_0011: br.s IL_0015 - - IL_0013: ldc.i4.0 - IL_0014: nop - IL_0015: brfalse.s IL_0019 + IL_0005: callvirt instance int32 [runtime]System.String::get_Length() + IL_000a: brtrue.s IL_000e - IL_0017: ldc.i4.0 - IL_0018: ret + IL_000c: ldc.i4.0 + IL_000d: ret - IL_0019: ldc.i4.1 - IL_001a: ret + IL_000e: ldc.i4.1 + IL_000f: ret } .method public static int32 testBundledEmptyAndNull(string s) cil managed @@ -121,27 +98,17 @@ .maxstack 8 IL_0000: nop IL_0001: ldarg.0 - IL_0002: brfalse.s IL_0010 + IL_0002: callvirt instance int32 [runtime]System.String::get_Length() + IL_0007: brfalse.s IL_000c - IL_0004: ldarg.0 - IL_0005: callvirt instance int32 [runtime]System.String::get_Length() - IL_000a: ldc.i4.0 - IL_000b: ceq - IL_000d: nop - IL_000e: br.s IL_0012 - - IL_0010: ldc.i4.0 - IL_0011: nop - IL_0012: brtrue.s IL_0017 - - IL_0014: ldarg.0 - IL_0015: brtrue.s IL_0019 + IL_0009: ldarg.0 + IL_000a: brtrue.s IL_000e - IL_0017: ldc.i4.0 - IL_0018: ret + IL_000c: ldc.i4.0 + IL_000d: ret - IL_0019: ldc.i4.1 - IL_001a: ret + IL_000e: ldc.i4.1 + IL_000f: ret } .method public static string useClassifyString(string s) cil managed @@ -150,32 +117,22 @@ .maxstack 8 IL_0000: nop IL_0001: ldarg.0 - IL_0002: brfalse.s IL_0010 + IL_0002: callvirt instance int32 [runtime]System.String::get_Length() + IL_0007: brfalse.s IL_000e - IL_0004: ldarg.0 - IL_0005: callvirt instance int32 [runtime]System.String::get_Length() - IL_000a: ldc.i4.0 - IL_000b: ceq - IL_000d: nop - IL_000e: br.s IL_0012 - - IL_0010: ldc.i4.0 - IL_0011: nop - IL_0012: brtrue.s IL_0019 + IL_0009: ldarg.0 + IL_000a: brfalse.s IL_0014 - IL_0014: ldarg.0 - IL_0015: brfalse.s IL_001f + IL_000c: br.s IL_001a - IL_0017: br.s IL_0025 + IL_000e: ldstr "empty" + IL_0013: ret - IL_0019: ldstr "empty" - IL_001e: ret + IL_0014: ldstr "null" + IL_0019: ret - IL_001f: ldstr "null" - IL_0024: ret - - IL_0025: ldstr "other" - IL_002a: ret + IL_001a: ldstr "other" + IL_001f: ret } .method public static int32 useTestEmptyStringOnly(string s) cil managed @@ -184,17 +141,14 @@ .maxstack 8 IL_0000: nop IL_0001: ldarg.0 - IL_0002: brfalse.s IL_000e - - IL_0004: ldarg.0 - IL_0005: callvirt instance int32 [runtime]System.String::get_Length() - IL_000a: brtrue.s IL_000e + IL_0002: callvirt instance int32 [runtime]System.String::get_Length() + IL_0007: brtrue.s IL_000b - IL_000c: ldc.i4.1 - IL_000d: ret + IL_0009: ldc.i4.1 + IL_000a: ret - IL_000e: ldc.i4.0 - IL_000f: ret + IL_000b: ldc.i4.0 + IL_000c: ret } .method public static int32 useBundledNullAndEmpty(string s) cil managed @@ -203,27 +157,17 @@ .maxstack 8 IL_0000: nop IL_0001: ldarg.0 - IL_0002: brfalse.s IL_0017 + IL_0002: brfalse.s IL_000c IL_0004: ldarg.0 - IL_0005: brfalse.s IL_0013 - - IL_0007: ldarg.0 - IL_0008: callvirt instance int32 [runtime]System.String::get_Length() - IL_000d: ldc.i4.0 - IL_000e: ceq - IL_0010: nop - IL_0011: br.s IL_0015 - - IL_0013: ldc.i4.0 - IL_0014: nop - IL_0015: brfalse.s IL_0019 + IL_0005: callvirt instance int32 [runtime]System.String::get_Length() + IL_000a: brtrue.s IL_000e - IL_0017: ldc.i4.0 - IL_0018: ret + IL_000c: ldc.i4.0 + IL_000d: ret - IL_0019: ldc.i4.1 - IL_001a: ret + IL_000e: ldc.i4.1 + IL_000f: ret } .method public static int32 useBundledEmptyAndNull(string s) cil managed @@ -232,27 +176,17 @@ .maxstack 8 IL_0000: nop IL_0001: ldarg.0 - IL_0002: brfalse.s IL_0010 + IL_0002: callvirt instance int32 [runtime]System.String::get_Length() + IL_0007: brfalse.s IL_000c - IL_0004: ldarg.0 - IL_0005: callvirt instance int32 [runtime]System.String::get_Length() - IL_000a: ldc.i4.0 - IL_000b: ceq - IL_000d: nop - IL_000e: br.s IL_0012 - - IL_0010: ldc.i4.0 - IL_0011: nop - IL_0012: brtrue.s IL_0017 - - IL_0014: ldarg.0 - IL_0015: brtrue.s IL_0019 + IL_0009: ldarg.0 + IL_000a: brtrue.s IL_000e - IL_0017: ldc.i4.0 - IL_0018: ret + IL_000c: ldc.i4.0 + IL_000d: ret - IL_0019: ldc.i4.1 - IL_001a: ret + IL_000e: ldc.i4.1 + IL_000f: ret } } From 09553a603978d07e0105439fa683a233f764173f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 7 Jan 2026 13:45:28 +0000 Subject: [PATCH 09/12] Remove debug output from BuildSwitch Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- src/Compiler/Checking/PatternMatchCompilation.fs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Compiler/Checking/PatternMatchCompilation.fs b/src/Compiler/Checking/PatternMatchCompilation.fs index 1370a496f92..8bfb6d5d469 100644 --- a/src/Compiler/Checking/PatternMatchCompilation.fs +++ b/src/Compiler/Checking/PatternMatchCompilation.fs @@ -768,7 +768,6 @@ let (|ConstNeedsDefaultCase|_|) c = /// switches, string switches and floating point switches are treated in the /// same way as DecisionTreeTest.IsInst. let rec BuildSwitch inpExprOpt g isNullFiltered expr edges dflt m = - if verbose then dprintf "--> BuildSwitch@%a, #edges = %A, dflt.IsSome = %A\n" outputRange m (List.length edges) (Option.isSome dflt) match edges, dflt with | [], None -> failwith "internal error: no edges and no default" | [], Some dflt -> dflt From 52d4206e547a3cae6b9896196b0cf92e499a6424 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 8 Jan 2026 09:13:18 +0000 Subject: [PATCH 10/12] Add debugging instrumentation to BuildSwitch for isNullFiltered investigation Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- .../Checking/PatternMatchCompilation.fs | 27 +++ .../EmittedIL/Inlining/DebugTest.fs.il.bsl | 180 ++++++++++++++++++ 2 files changed, 207 insertions(+) create mode 100644 tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/DebugTest.fs.il.bsl diff --git a/src/Compiler/Checking/PatternMatchCompilation.fs b/src/Compiler/Checking/PatternMatchCompilation.fs index 8bfb6d5d469..b857de99122 100644 --- a/src/Compiler/Checking/PatternMatchCompilation.fs +++ b/src/Compiler/Checking/PatternMatchCompilation.fs @@ -28,6 +28,9 @@ exception MatchIncomplete of bool * (string * bool) option * range exception RuleNeverMatched of range exception EnumMatchIncomplete of bool * (string * bool) option * range +// Debug flag for pattern match compilation - set to true to enable debug output +let verbose = false + type ActionOnFailure = | ThrowIncompleteMatchException | IgnoreWithWarning @@ -768,6 +771,17 @@ let (|ConstNeedsDefaultCase|_|) c = /// switches, string switches and floating point switches are treated in the /// same way as DecisionTreeTest.IsInst. let rec BuildSwitch inpExprOpt g isNullFiltered expr edges dflt m = + if verbose then + dprintf "BuildSwitch: isNullFiltered=%b, #edges=%d, dflt.IsSome=%b\n" isNullFiltered (List.length edges) (Option.isSome dflt) + edges |> List.iteri (fun i (TCase(discrim, _)) -> + let discrimStr = + match discrim with + | DecisionTreeTest.IsNull -> "IsNull" + | DecisionTreeTest.Const (Const.String s) -> sprintf "Const(String \"%s\")" s + | DecisionTreeTest.Const c -> sprintf "Const(%A)" c + | DecisionTreeTest.ArrayLength (n, _) -> sprintf "ArrayLength(%d)" n + | _ -> sprintf "%A" discrim + dprintf " Edge[%d]: %s\n" i discrimStr) match edges, dflt with | [], None -> failwith "internal error: no edges and no default" | [], Some dflt -> dflt @@ -793,23 +807,36 @@ let rec BuildSwitch inpExprOpt g isNullFiltered expr edges dflt m = // Split string, float, uint64, int64, unativeint, nativeint matches into serial equality tests | TCase((DecisionTreeTest.ArrayLength _ | DecisionTreeTest.Const (Const.Single _ | Const.Double _ | Const.String _ | Const.Decimal _ | Const.Int64 _ | Const.UInt64 _ | Const.IntPtr _ | Const.UIntPtr _)), _) :: _, Some dflt -> + if verbose then dprintf "foldBack: Processing %d edges with isNullFiltered=%b\n" (List.length edges) isNullFiltered List.foldBack (fun (TCase(discrim, tree)) sofar -> + if verbose then + let discrimStr = + match discrim with + | DecisionTreeTest.Const (Const.String s) -> sprintf "Const(String \"%s\")" s + | DecisionTreeTest.Const c -> sprintf "Const(%A)" c + | DecisionTreeTest.ArrayLength (n, _) -> sprintf "ArrayLength(%d)" n + | _ -> sprintf "%A" discrim + dprintf " foldBack iteration: discrim=%s, isNullFiltered=%b\n" discrimStr isNullFiltered let testexpr = expr let testexpr = match discrim with | DecisionTreeTest.ArrayLength(n, _) -> + if verbose then dprintf " ArrayLength: creating test with isNullFiltered=%b\n" isNullFiltered let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr // Skip null check if we're in a null-filtered context let test = mkILAsmCeq g m (mkLdlen g m vExpr) (mkInt g m n) let finalTest = if isNullFiltered then test else mkLazyAnd g m (mkNonNullTest g m vExpr) test + if verbose then dprintf " ArrayLength: skipping null check? %b\n" isNullFiltered mkLetBind m bind finalTest | DecisionTreeTest.Const (Const.String "") -> + if verbose then dprintf " Empty string: creating test with isNullFiltered=%b\n" isNullFiltered // Optimize empty string check to use null-safe length check let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr let test = mkILAsmCeq g m (mkGetStringLength g m vExpr) (mkInt g m 0) // Skip null check if we're in a null-filtered context let finalTest = if isNullFiltered then test else mkLazyAnd g m (mkNonNullTest g m vExpr) test + if verbose then dprintf " Empty string: skipping null check? %b\n" isNullFiltered mkLetBind m bind finalTest | DecisionTreeTest.Const (Const.String _ as c) -> mkCallEqualsOperator g m g.string_ty testexpr (Expr.Const (c, m, g.string_ty)) diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/DebugTest.fs.il.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/DebugTest.fs.il.bsl new file mode 100644 index 00000000000..2e560f5d5ca --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/DebugTest.fs.il.bsl @@ -0,0 +1,180 @@ + + + + + +.assembly extern runtime { } +.assembly extern FSharp.Core { } +.assembly assembly +{ + .custom instance void [FSharp.Core]Microsoft.FSharp.Core.FSharpInterfaceDataVersionAttribute::.ctor(int32, + int32, + int32) = ( 01 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 ) + + + + + .hash algorithm 0x00008004 + .ver 0:0:0:0 +} +.module assembly.exe + +.imagebase {value} +.file alignment 0x00000200 +.stackreserve 0x00100000 +.subsystem 0x0003 +.corflags 0x00000001 + + + + + +.class public abstract auto ansi sealed assembly + extends [runtime]System.Object +{ + .custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags) = ( 01 00 07 00 00 00 00 00 ) + .method public static int32 orPattern(string x) cil managed + { + + .maxstack 8 + IL_0000: nop + IL_0001: ldarg.0 + IL_0002: brfalse.s IL_0017 + + IL_0004: ldarg.0 + IL_0005: brfalse.s IL_0013 + + IL_0007: ldarg.0 + IL_0008: callvirt instance int32 [runtime]System.String::get_Length() + IL_000d: ldc.i4.0 + IL_000e: ceq + IL_0010: nop + IL_0011: br.s IL_0015 + + IL_0013: ldc.i4.0 + IL_0014: nop + IL_0015: brfalse.s IL_0019 + + IL_0017: ldc.i4.0 + IL_0018: ret + + IL_0019: ldc.i4.2 + IL_001a: ret + } + + .method public static int32 separateClauses(string x) cil managed + { + + .maxstack 8 + IL_0000: nop + IL_0001: ldarg.0 + IL_0002: brfalse.s IL_0019 + + IL_0004: ldarg.0 + IL_0005: brfalse.s IL_0013 + + IL_0007: ldarg.0 + IL_0008: callvirt instance int32 [runtime]System.String::get_Length() + IL_000d: ldc.i4.0 + IL_000e: ceq + IL_0010: nop + IL_0011: br.s IL_0015 + + IL_0013: ldc.i4.0 + IL_0014: nop + IL_0015: brfalse.s IL_001d + + IL_0017: br.s IL_001b + + IL_0019: ldc.i4.0 + IL_001a: ret + + IL_001b: ldc.i4.1 + IL_001c: ret + + IL_001d: ldc.i4.2 + IL_001e: ret + } + + .method public static int32 useOr(string x) cil managed + { + + .maxstack 8 + IL_0000: nop + IL_0001: ldarg.0 + IL_0002: brfalse.s IL_0017 + + IL_0004: ldarg.0 + IL_0005: brfalse.s IL_0013 + + IL_0007: ldarg.0 + IL_0008: callvirt instance int32 [runtime]System.String::get_Length() + IL_000d: ldc.i4.0 + IL_000e: ceq + IL_0010: nop + IL_0011: br.s IL_0015 + + IL_0013: ldc.i4.0 + IL_0014: nop + IL_0015: brfalse.s IL_0019 + + IL_0017: ldc.i4.0 + IL_0018: ret + + IL_0019: ldc.i4.2 + IL_001a: ret + } + + .method public static int32 useSeparate(string x) cil managed + { + + .maxstack 8 + IL_0000: nop + IL_0001: ldarg.0 + IL_0002: brfalse.s IL_0019 + + IL_0004: ldarg.0 + IL_0005: brfalse.s IL_0013 + + IL_0007: ldarg.0 + IL_0008: callvirt instance int32 [runtime]System.String::get_Length() + IL_000d: ldc.i4.0 + IL_000e: ceq + IL_0010: nop + IL_0011: br.s IL_0015 + + IL_0013: ldc.i4.0 + IL_0014: nop + IL_0015: brfalse.s IL_001d + + IL_0017: br.s IL_001b + + IL_0019: ldc.i4.0 + IL_001a: ret + + IL_001b: ldc.i4.1 + IL_001c: ret + + IL_001d: ldc.i4.2 + IL_001e: ret + } + +} + +.class private abstract auto ansi sealed ''.$assembly + extends [runtime]System.Object +{ + .method public static void main@() cil managed + { + .entrypoint + + .maxstack 8 + IL_0000: ret + } + +} + + + + + From 68bc4c1567ba30bb13995ffd98b509bbda98143b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 9 Jan 2026 10:33:26 +0000 Subject: [PATCH 11/12] Add check for IsNull edge in foldBack to attempt eliminating redundant null checks Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- .../Checking/PatternMatchCompilation.fs | 28 ++- .../EmittedIL/Inlining/DebugTest.fs.il.bsl | 180 ---------------- .../Inlining/EmptyStringPattern.fs.il.bsl | 194 ++++++++++++------ 3 files changed, 150 insertions(+), 252 deletions(-) delete mode 100644 tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/DebugTest.fs.il.bsl diff --git a/src/Compiler/Checking/PatternMatchCompilation.fs b/src/Compiler/Checking/PatternMatchCompilation.fs index b857de99122..3745f960d06 100644 --- a/src/Compiler/Checking/PatternMatchCompilation.fs +++ b/src/Compiler/Checking/PatternMatchCompilation.fs @@ -799,6 +799,15 @@ let rec BuildSwitch inpExprOpt g isNullFiltered expr edges dflt m = | TCase((DecisionTreeTest.IsNull | DecisionTreeTest.IsInst _), _) as edge :: edges, dflt -> // After an IsNull test, in the fallthrough branch (Some), we know the value is not null let nullFiltered = match edge with TCase(DecisionTreeTest.IsNull, _) -> true | _ -> isNullFiltered + if verbose then + dprintf "IsNull/IsInst: edge type=%s, remaining edges=%d, setting nullFiltered=%b\n" + (match edge with | TCase(DecisionTreeTest.IsNull, _) -> "IsNull" | _ -> "IsInst") + (List.length edges) nullFiltered + edges |> List.iteri (fun i (TCase(d, _)) -> + let dStr = match d with + | DecisionTreeTest.Const (Const.String s) -> sprintf "Const(String \"%s\")" s + | _ -> sprintf "%A" d + dprintf " Remaining edge[%d]: %s\n" i dStr) TDSwitch(expr, [edge], Some (BuildSwitch None g nullFiltered expr edges dflt m), m) // All these should also always have default cases @@ -808,6 +817,10 @@ let rec BuildSwitch inpExprOpt g isNullFiltered expr edges dflt m = // Split string, float, uint64, int64, unativeint, nativeint matches into serial equality tests | TCase((DecisionTreeTest.ArrayLength _ | DecisionTreeTest.Const (Const.Single _ | Const.Double _ | Const.String _ | Const.Decimal _ | Const.Int64 _ | Const.UInt64 _ | Const.IntPtr _ | Const.UIntPtr _)), _) :: _, Some dflt -> if verbose then dprintf "foldBack: Processing %d edges with isNullFiltered=%b\n" (List.length edges) isNullFiltered + // Check if any edge is an IsNull test - if so, we don't need null checks for subsequent tests + let hasIsNullEdge = edges |> List.exists (fun (TCase(d, _)) -> match d with | DecisionTreeTest.IsNull -> true | _ -> false) + let effectiveIsNullFiltered = isNullFiltered || hasIsNullEdge + if verbose && hasIsNullEdge then dprintf " Found IsNull edge in list, setting effectiveIsNullFiltered=true\n" List.foldBack (fun (TCase(discrim, tree)) sofar -> if verbose then @@ -817,26 +830,25 @@ let rec BuildSwitch inpExprOpt g isNullFiltered expr edges dflt m = | DecisionTreeTest.Const c -> sprintf "Const(%A)" c | DecisionTreeTest.ArrayLength (n, _) -> sprintf "ArrayLength(%d)" n | _ -> sprintf "%A" discrim - dprintf " foldBack iteration: discrim=%s, isNullFiltered=%b\n" discrimStr isNullFiltered + dprintf " foldBack iteration: discrim=%s, effectiveIsNullFiltered=%b\n" discrimStr effectiveIsNullFiltered let testexpr = expr let testexpr = match discrim with | DecisionTreeTest.ArrayLength(n, _) -> - if verbose then dprintf " ArrayLength: creating test with isNullFiltered=%b\n" isNullFiltered + if verbose then dprintf " ArrayLength: creating test with effectiveIsNullFiltered=%b\n" effectiveIsNullFiltered let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr // Skip null check if we're in a null-filtered context let test = mkILAsmCeq g m (mkLdlen g m vExpr) (mkInt g m n) - let finalTest = if isNullFiltered then test else mkLazyAnd g m (mkNonNullTest g m vExpr) test - if verbose then dprintf " ArrayLength: skipping null check? %b\n" isNullFiltered + let finalTest = if effectiveIsNullFiltered then test else mkLazyAnd g m (mkNonNullTest g m vExpr) test + if verbose then dprintf " ArrayLength: skipping null check? %b\n" effectiveIsNullFiltered mkLetBind m bind finalTest | DecisionTreeTest.Const (Const.String "") -> - if verbose then dprintf " Empty string: creating test with isNullFiltered=%b\n" isNullFiltered // Optimize empty string check to use null-safe length check let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr let test = mkILAsmCeq g m (mkGetStringLength g m vExpr) (mkInt g m 0) - // Skip null check if we're in a null-filtered context - let finalTest = if isNullFiltered then test else mkLazyAnd g m (mkNonNullTest g m vExpr) test - if verbose then dprintf " Empty string: skipping null check? %b\n" isNullFiltered + // Skip null check if we're in a null-filtered context OR if there's an IsNull edge + let finalTest = if effectiveIsNullFiltered then test else mkLazyAnd g m (mkNonNullTest g m vExpr) test + if verbose then dprintf " Empty string: skipping null check? %b\n" effectiveIsNullFiltered mkLetBind m bind finalTest | DecisionTreeTest.Const (Const.String _ as c) -> mkCallEqualsOperator g m g.string_ty testexpr (Expr.Const (c, m, g.string_ty)) diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/DebugTest.fs.il.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/DebugTest.fs.il.bsl deleted file mode 100644 index 2e560f5d5ca..00000000000 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/DebugTest.fs.il.bsl +++ /dev/null @@ -1,180 +0,0 @@ - - - - - -.assembly extern runtime { } -.assembly extern FSharp.Core { } -.assembly assembly -{ - .custom instance void [FSharp.Core]Microsoft.FSharp.Core.FSharpInterfaceDataVersionAttribute::.ctor(int32, - int32, - int32) = ( 01 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 ) - - - - - .hash algorithm 0x00008004 - .ver 0:0:0:0 -} -.module assembly.exe - -.imagebase {value} -.file alignment 0x00000200 -.stackreserve 0x00100000 -.subsystem 0x0003 -.corflags 0x00000001 - - - - - -.class public abstract auto ansi sealed assembly - extends [runtime]System.Object -{ - .custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags) = ( 01 00 07 00 00 00 00 00 ) - .method public static int32 orPattern(string x) cil managed - { - - .maxstack 8 - IL_0000: nop - IL_0001: ldarg.0 - IL_0002: brfalse.s IL_0017 - - IL_0004: ldarg.0 - IL_0005: brfalse.s IL_0013 - - IL_0007: ldarg.0 - IL_0008: callvirt instance int32 [runtime]System.String::get_Length() - IL_000d: ldc.i4.0 - IL_000e: ceq - IL_0010: nop - IL_0011: br.s IL_0015 - - IL_0013: ldc.i4.0 - IL_0014: nop - IL_0015: brfalse.s IL_0019 - - IL_0017: ldc.i4.0 - IL_0018: ret - - IL_0019: ldc.i4.2 - IL_001a: ret - } - - .method public static int32 separateClauses(string x) cil managed - { - - .maxstack 8 - IL_0000: nop - IL_0001: ldarg.0 - IL_0002: brfalse.s IL_0019 - - IL_0004: ldarg.0 - IL_0005: brfalse.s IL_0013 - - IL_0007: ldarg.0 - IL_0008: callvirt instance int32 [runtime]System.String::get_Length() - IL_000d: ldc.i4.0 - IL_000e: ceq - IL_0010: nop - IL_0011: br.s IL_0015 - - IL_0013: ldc.i4.0 - IL_0014: nop - IL_0015: brfalse.s IL_001d - - IL_0017: br.s IL_001b - - IL_0019: ldc.i4.0 - IL_001a: ret - - IL_001b: ldc.i4.1 - IL_001c: ret - - IL_001d: ldc.i4.2 - IL_001e: ret - } - - .method public static int32 useOr(string x) cil managed - { - - .maxstack 8 - IL_0000: nop - IL_0001: ldarg.0 - IL_0002: brfalse.s IL_0017 - - IL_0004: ldarg.0 - IL_0005: brfalse.s IL_0013 - - IL_0007: ldarg.0 - IL_0008: callvirt instance int32 [runtime]System.String::get_Length() - IL_000d: ldc.i4.0 - IL_000e: ceq - IL_0010: nop - IL_0011: br.s IL_0015 - - IL_0013: ldc.i4.0 - IL_0014: nop - IL_0015: brfalse.s IL_0019 - - IL_0017: ldc.i4.0 - IL_0018: ret - - IL_0019: ldc.i4.2 - IL_001a: ret - } - - .method public static int32 useSeparate(string x) cil managed - { - - .maxstack 8 - IL_0000: nop - IL_0001: ldarg.0 - IL_0002: brfalse.s IL_0019 - - IL_0004: ldarg.0 - IL_0005: brfalse.s IL_0013 - - IL_0007: ldarg.0 - IL_0008: callvirt instance int32 [runtime]System.String::get_Length() - IL_000d: ldc.i4.0 - IL_000e: ceq - IL_0010: nop - IL_0011: br.s IL_0015 - - IL_0013: ldc.i4.0 - IL_0014: nop - IL_0015: brfalse.s IL_001d - - IL_0017: br.s IL_001b - - IL_0019: ldc.i4.0 - IL_001a: ret - - IL_001b: ldc.i4.1 - IL_001c: ret - - IL_001d: ldc.i4.2 - IL_001e: ret - } - -} - -.class private abstract auto ansi sealed ''.$assembly - extends [runtime]System.Object -{ - .method public static void main@() cil managed - { - .entrypoint - - .maxstack 8 - IL_0000: ret - } - -} - - - - - diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs.il.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs.il.bsl index 51cf669e07c..0d1c3c137a5 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs.il.bsl +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/EmptyStringPattern.fs.il.bsl @@ -39,22 +39,32 @@ .maxstack 8 IL_0000: nop IL_0001: ldarg.0 - IL_0002: callvirt instance int32 [runtime]System.String::get_Length() - IL_0007: brfalse.s IL_000e + IL_0002: brfalse.s IL_0010 - IL_0009: ldarg.0 - IL_000a: brfalse.s IL_0014 + IL_0004: ldarg.0 + IL_0005: callvirt instance int32 [runtime]System.String::get_Length() + IL_000a: ldc.i4.0 + IL_000b: ceq + IL_000d: nop + IL_000e: br.s IL_0012 + + IL_0010: ldc.i4.0 + IL_0011: nop + IL_0012: brtrue.s IL_0019 - IL_000c: br.s IL_001a + IL_0014: ldarg.0 + IL_0015: brfalse.s IL_001f - IL_000e: ldstr "empty" - IL_0013: ret + IL_0017: br.s IL_0025 - IL_0014: ldstr "null" - IL_0019: ret + IL_0019: ldstr "empty" + IL_001e: ret - IL_001a: ldstr "other" - IL_001f: ret + IL_001f: ldstr "null" + IL_0024: ret + + IL_0025: ldstr "other" + IL_002a: ret } .method public static int32 testEmptyStringOnly(string s) cil managed @@ -63,14 +73,17 @@ .maxstack 8 IL_0000: nop IL_0001: ldarg.0 - IL_0002: callvirt instance int32 [runtime]System.String::get_Length() - IL_0007: brtrue.s IL_000b + IL_0002: brfalse.s IL_000e + + IL_0004: ldarg.0 + IL_0005: callvirt instance int32 [runtime]System.String::get_Length() + IL_000a: brtrue.s IL_000e - IL_0009: ldc.i4.1 - IL_000a: ret + IL_000c: ldc.i4.1 + IL_000d: ret - IL_000b: ldc.i4.0 - IL_000c: ret + IL_000e: ldc.i4.0 + IL_000f: ret } .method public static int32 testBundledNullAndEmpty(string s) cil managed @@ -79,17 +92,27 @@ .maxstack 8 IL_0000: nop IL_0001: ldarg.0 - IL_0002: brfalse.s IL_000c + IL_0002: brfalse.s IL_0017 IL_0004: ldarg.0 - IL_0005: callvirt instance int32 [runtime]System.String::get_Length() - IL_000a: brtrue.s IL_000e + IL_0005: brfalse.s IL_0013 - IL_000c: ldc.i4.0 - IL_000d: ret + IL_0007: ldarg.0 + IL_0008: callvirt instance int32 [runtime]System.String::get_Length() + IL_000d: ldc.i4.0 + IL_000e: ceq + IL_0010: nop + IL_0011: br.s IL_0015 - IL_000e: ldc.i4.1 - IL_000f: ret + IL_0013: ldc.i4.0 + IL_0014: nop + IL_0015: brfalse.s IL_0019 + + IL_0017: ldc.i4.0 + IL_0018: ret + + IL_0019: ldc.i4.1 + IL_001a: ret } .method public static int32 testBundledEmptyAndNull(string s) cil managed @@ -98,17 +121,27 @@ .maxstack 8 IL_0000: nop IL_0001: ldarg.0 - IL_0002: callvirt instance int32 [runtime]System.String::get_Length() - IL_0007: brfalse.s IL_000c + IL_0002: brfalse.s IL_0010 - IL_0009: ldarg.0 - IL_000a: brtrue.s IL_000e + IL_0004: ldarg.0 + IL_0005: callvirt instance int32 [runtime]System.String::get_Length() + IL_000a: ldc.i4.0 + IL_000b: ceq + IL_000d: nop + IL_000e: br.s IL_0012 - IL_000c: ldc.i4.0 - IL_000d: ret + IL_0010: ldc.i4.0 + IL_0011: nop + IL_0012: brtrue.s IL_0017 - IL_000e: ldc.i4.1 - IL_000f: ret + IL_0014: ldarg.0 + IL_0015: brtrue.s IL_0019 + + IL_0017: ldc.i4.0 + IL_0018: ret + + IL_0019: ldc.i4.1 + IL_001a: ret } .method public static string useClassifyString(string s) cil managed @@ -117,22 +150,32 @@ .maxstack 8 IL_0000: nop IL_0001: ldarg.0 - IL_0002: callvirt instance int32 [runtime]System.String::get_Length() - IL_0007: brfalse.s IL_000e + IL_0002: brfalse.s IL_0010 - IL_0009: ldarg.0 - IL_000a: brfalse.s IL_0014 + IL_0004: ldarg.0 + IL_0005: callvirt instance int32 [runtime]System.String::get_Length() + IL_000a: ldc.i4.0 + IL_000b: ceq + IL_000d: nop + IL_000e: br.s IL_0012 + + IL_0010: ldc.i4.0 + IL_0011: nop + IL_0012: brtrue.s IL_0019 - IL_000c: br.s IL_001a + IL_0014: ldarg.0 + IL_0015: brfalse.s IL_001f - IL_000e: ldstr "empty" - IL_0013: ret + IL_0017: br.s IL_0025 - IL_0014: ldstr "null" - IL_0019: ret + IL_0019: ldstr "empty" + IL_001e: ret - IL_001a: ldstr "other" - IL_001f: ret + IL_001f: ldstr "null" + IL_0024: ret + + IL_0025: ldstr "other" + IL_002a: ret } .method public static int32 useTestEmptyStringOnly(string s) cil managed @@ -141,14 +184,17 @@ .maxstack 8 IL_0000: nop IL_0001: ldarg.0 - IL_0002: callvirt instance int32 [runtime]System.String::get_Length() - IL_0007: brtrue.s IL_000b + IL_0002: brfalse.s IL_000e + + IL_0004: ldarg.0 + IL_0005: callvirt instance int32 [runtime]System.String::get_Length() + IL_000a: brtrue.s IL_000e - IL_0009: ldc.i4.1 - IL_000a: ret + IL_000c: ldc.i4.1 + IL_000d: ret - IL_000b: ldc.i4.0 - IL_000c: ret + IL_000e: ldc.i4.0 + IL_000f: ret } .method public static int32 useBundledNullAndEmpty(string s) cil managed @@ -157,17 +203,27 @@ .maxstack 8 IL_0000: nop IL_0001: ldarg.0 - IL_0002: brfalse.s IL_000c + IL_0002: brfalse.s IL_0017 IL_0004: ldarg.0 - IL_0005: callvirt instance int32 [runtime]System.String::get_Length() - IL_000a: brtrue.s IL_000e + IL_0005: brfalse.s IL_0013 - IL_000c: ldc.i4.0 - IL_000d: ret + IL_0007: ldarg.0 + IL_0008: callvirt instance int32 [runtime]System.String::get_Length() + IL_000d: ldc.i4.0 + IL_000e: ceq + IL_0010: nop + IL_0011: br.s IL_0015 - IL_000e: ldc.i4.1 - IL_000f: ret + IL_0013: ldc.i4.0 + IL_0014: nop + IL_0015: brfalse.s IL_0019 + + IL_0017: ldc.i4.0 + IL_0018: ret + + IL_0019: ldc.i4.1 + IL_001a: ret } .method public static int32 useBundledEmptyAndNull(string s) cil managed @@ -176,17 +232,27 @@ .maxstack 8 IL_0000: nop IL_0001: ldarg.0 - IL_0002: callvirt instance int32 [runtime]System.String::get_Length() - IL_0007: brfalse.s IL_000c + IL_0002: brfalse.s IL_0010 - IL_0009: ldarg.0 - IL_000a: brtrue.s IL_000e + IL_0004: ldarg.0 + IL_0005: callvirt instance int32 [runtime]System.String::get_Length() + IL_000a: ldc.i4.0 + IL_000b: ceq + IL_000d: nop + IL_000e: br.s IL_0012 - IL_000c: ldc.i4.0 - IL_000d: ret + IL_0010: ldc.i4.0 + IL_0011: nop + IL_0012: brtrue.s IL_0017 - IL_000e: ldc.i4.1 - IL_000f: ret + IL_0014: ldarg.0 + IL_0015: brtrue.s IL_0019 + + IL_0017: ldc.i4.0 + IL_0018: ret + + IL_0019: ldc.i4.1 + IL_001a: ret } } From cdbd93d08d75d4fbe3ecc03f9dcfb6b80ba3bcd4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 9 Jan 2026 11:01:31 +0000 Subject: [PATCH 12/12] Simplify isNullFiltered logic - remove hasIsNullEdge computation Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- .../Checking/PatternMatchCompilation.fs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/Compiler/Checking/PatternMatchCompilation.fs b/src/Compiler/Checking/PatternMatchCompilation.fs index 3745f960d06..033f8572bbe 100644 --- a/src/Compiler/Checking/PatternMatchCompilation.fs +++ b/src/Compiler/Checking/PatternMatchCompilation.fs @@ -817,10 +817,6 @@ let rec BuildSwitch inpExprOpt g isNullFiltered expr edges dflt m = // Split string, float, uint64, int64, unativeint, nativeint matches into serial equality tests | TCase((DecisionTreeTest.ArrayLength _ | DecisionTreeTest.Const (Const.Single _ | Const.Double _ | Const.String _ | Const.Decimal _ | Const.Int64 _ | Const.UInt64 _ | Const.IntPtr _ | Const.UIntPtr _)), _) :: _, Some dflt -> if verbose then dprintf "foldBack: Processing %d edges with isNullFiltered=%b\n" (List.length edges) isNullFiltered - // Check if any edge is an IsNull test - if so, we don't need null checks for subsequent tests - let hasIsNullEdge = edges |> List.exists (fun (TCase(d, _)) -> match d with | DecisionTreeTest.IsNull -> true | _ -> false) - let effectiveIsNullFiltered = isNullFiltered || hasIsNullEdge - if verbose && hasIsNullEdge then dprintf " Found IsNull edge in list, setting effectiveIsNullFiltered=true\n" List.foldBack (fun (TCase(discrim, tree)) sofar -> if verbose then @@ -830,25 +826,25 @@ let rec BuildSwitch inpExprOpt g isNullFiltered expr edges dflt m = | DecisionTreeTest.Const c -> sprintf "Const(%A)" c | DecisionTreeTest.ArrayLength (n, _) -> sprintf "ArrayLength(%d)" n | _ -> sprintf "%A" discrim - dprintf " foldBack iteration: discrim=%s, effectiveIsNullFiltered=%b\n" discrimStr effectiveIsNullFiltered + dprintf " foldBack iteration: discrim=%s, isNullFiltered=%b\n" discrimStr isNullFiltered let testexpr = expr let testexpr = match discrim with | DecisionTreeTest.ArrayLength(n, _) -> - if verbose then dprintf " ArrayLength: creating test with effectiveIsNullFiltered=%b\n" effectiveIsNullFiltered + if verbose then dprintf " ArrayLength: creating test with isNullFiltered=%b\n" isNullFiltered let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr // Skip null check if we're in a null-filtered context let test = mkILAsmCeq g m (mkLdlen g m vExpr) (mkInt g m n) - let finalTest = if effectiveIsNullFiltered then test else mkLazyAnd g m (mkNonNullTest g m vExpr) test - if verbose then dprintf " ArrayLength: skipping null check? %b\n" effectiveIsNullFiltered + let finalTest = if isNullFiltered then test else mkLazyAnd g m (mkNonNullTest g m vExpr) test + if verbose then dprintf " ArrayLength: skipping null check? %b\n" isNullFiltered mkLetBind m bind finalTest | DecisionTreeTest.Const (Const.String "") -> // Optimize empty string check to use null-safe length check let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr let test = mkILAsmCeq g m (mkGetStringLength g m vExpr) (mkInt g m 0) - // Skip null check if we're in a null-filtered context OR if there's an IsNull edge - let finalTest = if effectiveIsNullFiltered then test else mkLazyAnd g m (mkNonNullTest g m vExpr) test - if verbose then dprintf " Empty string: skipping null check? %b\n" effectiveIsNullFiltered + // Skip null check if we're in a null-filtered context + let finalTest = if isNullFiltered then test else mkLazyAnd g m (mkNonNullTest g m vExpr) test + if verbose then dprintf " Empty string: skipping null check? %b\n" isNullFiltered mkLetBind m bind finalTest | DecisionTreeTest.Const (Const.String _ as c) -> mkCallEqualsOperator g m g.string_ty testexpr (Expr.Const (c, m, g.string_ty))