From c0150aa432447e6a87fd899916ad0e7641832236 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Thu, 14 May 2026 16:53:12 +0200 Subject: [PATCH 1/3] Fix nullness narrowing for match inside seq/list/array comprehensions (#19644) Extract null-narrowing helper EliminateNullnessFromInputType from TcMatchClause and reuse it in TcSequenceExpression's SynExpr.Match handler so the input type is narrowed across clauses inside comprehensions, matching the behavior of plain match expressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Checking/Expressions/CheckExpressions.fs | 67 ++++++++++--------- .../Checking/Expressions/CheckExpressions.fsi | 5 ++ .../Expressions/CheckSequenceExpressions.fs | 10 +-- .../Nullness/NullableRegressionTests.fs | 64 ++++++++++++++++++ 4 files changed, 111 insertions(+), 35 deletions(-) diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs index 30d49e776c8..a2abd7ec83d 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckExpressions.fs @@ -4125,6 +4125,41 @@ let formatAvailableNames (names: string array) = let result = truncated |> String.concat ", " if names.Length > 5 then result + ", ..." else result +/// Given the current 'inputTy' of a match clause, the elaborated pattern, and the optional 'when' clause, +/// returns the (possibly narrowed) inputTy to use for subsequent clauses. +/// E.g. `match x with | null -> ... | y -> ...` narrows `inputTy` of the y-clause to non-null. +let EliminateNullnessFromInputType (g: TcGlobals) (inputTy: TType) (pat: Pattern) (whenExprOpt: Expr option) : TType = + let removeNull t = + // Strip type equations (including abbreviations) and set nullness to non-null. + // For type abbreviations like `type objnull = obj | null`, we need to expand + // the abbreviation and apply non-null to the underlying type. + let stripped = stripTyEqns g t + replaceNullnessOfTy KnownWithoutNull stripped + let rec isWild (p: Pattern) = + match p with + | TPat_wild _ -> true + | TPat_as (p,_,_) -> isWild p + | TPat_disjs(patterns,_) -> patterns |> List.exists isWild + | TPat_conjs(patterns,_) -> patterns |> List.forall isWild + | TPat_tuple (_,pats,_,_) -> pats |> List.forall isWild + | _ -> false + + let rec eliminateNull (ty: TType) (p: Pattern) = + match p with + | TPat_null _ -> removeNull ty + | TPat_as (p,_,_) -> eliminateNull ty p + | TPat_disjs(patterns,_) -> (ty,patterns) ||> List.fold eliminateNull + | TPat_tuple (_,pats,_,_) -> + match stripTyparEqns ty with + // In a tuple, if 1 elem is matched for null and the rest are wild => subsequent clauses can strip nullness + | TType_tuple(ti,tys) when tys.Length = pats.Length && (pats |> List.count (isWild >> not)) = 1 -> + TType_tuple(ti, List.map2 eliminateNull tys pats) + | _ -> ty + | _ -> ty + match whenExprOpt with + | None -> eliminateNull inputTy pat + | _ -> inputTy + //------------------------------------------------------------------------- // Checking types and type constraints //------------------------------------------------------------------------- @@ -10967,37 +11002,7 @@ and TcMatchClause cenv inputTy (resultTy: OverallTy) env isFirst tpenv synMatchC let target = TTarget(vspecs, resultExpr, None) - let inputTypeForNextPatterns= - let removeNull t = - // Strip type equations (including abbreviations) and set nullness to non-null. - // For type abbreviations like `type objnull = obj | null`, we need to expand - // the abbreviation and apply non-null to the underlying type. - let stripped = stripTyEqns cenv.g t - replaceNullnessOfTy KnownWithoutNull stripped - let rec isWild (p:Pattern) = - match p with - | TPat_wild _ -> true - | TPat_as (p,_,_) -> isWild p - | TPat_disjs(patterns,_) -> patterns |> List.exists isWild - | TPat_conjs(patterns,_) -> patterns |> List.forall isWild - | TPat_tuple (_,pats,_,_) -> pats |> List.forall isWild - | _ -> false - - let rec eliminateNull (ty:TType) (p:Pattern) = - match p with - | TPat_null _ -> removeNull ty - | TPat_as (p,_,_) -> eliminateNull ty p - | TPat_disjs(patterns,_) -> (ty,patterns) ||> List.fold eliminateNull - | TPat_tuple (_,pats,_,_) -> - match stripTyparEqns ty with - // In a tuple, if 1 elem is matched for null and the rest are wild => subsequent clauses can strip nullness - | TType_tuple(ti,tys) when tys.Length = pats.Length && (pats |> List.count (isWild >> not)) = 1 -> - TType_tuple(ti, List.map2 eliminateNull tys pats) - | _ -> ty - | _ -> ty - match whenExprOpt with - | None -> eliminateNull inputTy pat - | _ -> inputTy + let inputTypeForNextPatterns = EliminateNullnessFromInputType cenv.g inputTy pat whenExprOpt MatchClause(pat, whenExprOpt, target, patm), (tpenv,inputTypeForNextPatterns) diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fsi b/src/Compiler/Checking/Expressions/CheckExpressions.fsi index d8f801c7c5c..d99bfa88e18 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fsi +++ b/src/Compiler/Checking/Expressions/CheckExpressions.fsi @@ -721,6 +721,11 @@ val TcMatchPattern: tcTrueMatchClause: TcTrueMatchClause -> Pattern * Expr option * Val list * TcEnv * UnscopedTyparEnv +/// Given the current 'inputTy' of a match clause, the elaborated pattern, and the optional 'when' clause, +/// returns the (possibly narrowed) inputTy to use for subsequent clauses. +/// E.g. `match x with | null -> ... | y -> ...` narrows `inputTy` of the y-clause to non-null. +val EliminateNullnessFromInputType: g: TcGlobals -> inputTy: TType -> pat: Pattern -> whenExprOpt: Expr option -> TType + [] val (|BinOpExpr|_|): SynExpr -> (Ident * SynExpr * SynExpr) voption diff --git a/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs b/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs index f8eeb4da5c1..86bf635d58a 100644 --- a/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs @@ -272,9 +272,9 @@ let TcSequenceExpression (cenv: TcFileState) env tpenv comp (overallTy: OverallT | SynExpr.Match(spMatch, expr, clauses, _m, _trivia) -> let inputExpr, inputTy, tpenv = TcExprOfUnknownType cenv env tpenv expr - let tclauses, tpenv = - (tpenv, clauses) - ||> List.mapFold (fun tpenv (SynMatchClause(pat, cond, innerComp, _, sp, _trivia) as clause) -> + let tclauses, (tpenv, _finalInputTy) = + ((tpenv, inputTy), clauses) + ||> List.mapFold (fun (tpenv, inputTy) (SynMatchClause(pat, cond, innerComp, _, sp, _trivia) as clause) -> let isTrueMatchClause = if clause.IsTrueMatchClause then TcTrueMatchClause.Yes @@ -290,7 +290,9 @@ let TcSequenceExpression (cenv: TcFileState) env tpenv comp (overallTy: OverallT | DebugPointAtTarget.No -> envinner let innerExpr, tpenv = tcSequenceExprBody envinner genOuterTy tpenv innerComp - MatchClause(patR, condR, TTarget(vspecs, innerExpr, None), patR.Range), tpenv) + + let nextInputTy = EliminateNullnessFromInputType cenv.g inputTy patR condR + MatchClause(patR, condR, TTarget(vspecs, innerExpr, None), patR.Range), (tpenv, nextInputTy)) let inputExprTy = tyOfExpr cenv.g inputExpr let inputExprMark = inputExpr.Range diff --git a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableRegressionTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableRegressionTests.fs index bd3d90a8074..32dc2b37361 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableRegressionTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableRegressionTests.fs @@ -198,3 +198,67 @@ type C7 = |> shouldFail |> withDiagnostics [(Error 444, Line 7, Col 17, Line 7, Col 23, "The type of a field using the 'DefaultValue' attribute must admit default initialization, i.e. have 'null' as a proper value or be a struct type whose fields all admit default initialization. You can use 'DefaultValue(false)' to disable this check")] + +[] +let ``Issue 19644 - match-null narrowing inside list comprehension`` () = + FSharp """ +module M +open System.Collections.Immutable + +let sets (builders: (ImmutableHashSet.Builder | null)[]) = [ + for builder in builders do + match builder with + | null -> () + | b -> yield b.ToImmutable() +] +""" + |> withVersionAndCheckNulls ("preview", true) + |> compile + |> shouldSucceed + +[] +let ``Issue 19644 - match-null narrowing inside seq expression`` () = + FSharp """ +module M +let v (xs: (string | null) seq) = + seq { + for x in xs do + match x with + | null -> () + | y -> yield y.Length + } +""" + |> withVersionAndCheckNulls ("preview", true) + |> compile + |> shouldSucceed + +[] +let ``Issue 19644 - match-null narrowing inside array comprehension`` () = + FSharp """ +module M +let v (xs: (string | null)[]) = [| + for x in xs do + match x with + | null -> () + | y -> yield y.Length +|] +""" + |> withVersionAndCheckNulls ("preview", true) + |> compile + |> shouldSucceed + +[] +let ``Issue 19644 - match-null narrowing inside list comprehension (no for)`` () = + FSharp """ +module M +open System.Collections.Immutable + +let v (b: ImmutableHashSet.Builder | null) = [ + match b with + | null -> () + | x -> yield x.ToImmutable() +] +""" + |> withVersionAndCheckNulls ("preview", true) + |> compile + |> shouldSucceed From 3c45d201fcced1cfc7716bcd29f57ef66ea5c738 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Fri, 15 May 2026 14:26:57 +0200 Subject: [PATCH 2/3] Add release notes for #19644 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/release-notes/.FSharp.Compiler.Service/11.0.100.md | 1 + 1 file changed, 1 insertion(+) 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 4a6110cf514..670fb7c2c9c 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,6 +1,7 @@ ### Fixed * Fix attributes on return type of unparenthesized tuple methods being silently dropped from IL. ([Issue #462](https://github.com/dotnet/fsharp/issues/462), [PR #19714](https://github.com/dotnet/fsharp/pull/19714)) +* Fix false-positive nullness warning (FS3261) when pattern matching narrows nullness inside seq/list/array comprehensions. ([Issue #19644](https://github.com/dotnet/fsharp/issues/19644), [PR #19743](https://github.com/dotnet/fsharp/pull/19743)) * Fix internal error FS0073 "Undefined or unsolved type variable" in IlxGen when nested inline SRTP functions with multiple overloads leave unsolved typars in the non-witness codegen path. ([Issue #19709](https://github.com/dotnet/fsharp/issues/19709), [PR #19710](https://github.com/dotnet/fsharp/pull/19710)) * Fix NRE when calling virtual Object methods on value types through inline SRTP functions. ([Issue #8098](https://github.com/dotnet/fsharp/issues/8098), [PR #19511](https://github.com/dotnet/fsharp/pull/19511)) * Fix attributes not resolved from opened namespaces in `namespace rec` / `module rec` scopes. ([Issue #7931](https://github.com/dotnet/fsharp/issues/7931), [PR #19502](https://github.com/dotnet/fsharp/pull/19502)) From 236fd2f3430c82fb3f229b3ec8962bbc3a1b8be3 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Tue, 19 May 2026 13:47:21 +0200 Subject: [PATCH 3/3] Fix test failures on net472: remove System.Collections.Immutable dependency The ImmutableHashSet.Builder type is not available in the net472 test compilation environment. Replace with string | null which tests the same nullness narrowing logic without requiring external assembly references. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Language/Nullness/NullableRegressionTests.fs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableRegressionTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableRegressionTests.fs index 32dc2b37361..a7ff6c2e89b 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableRegressionTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableRegressionTests.fs @@ -203,13 +203,12 @@ type C7 = let ``Issue 19644 - match-null narrowing inside list comprehension`` () = FSharp """ module M -open System.Collections.Immutable -let sets (builders: (ImmutableHashSet.Builder | null)[]) = [ - for builder in builders do - match builder with +let lengths (xs: (string | null)[]) = [ + for x in xs do + match x with | null -> () - | b -> yield b.ToImmutable() + | s -> yield s.Length ] """ |> withVersionAndCheckNulls ("preview", true) @@ -251,12 +250,11 @@ let v (xs: (string | null)[]) = [| let ``Issue 19644 - match-null narrowing inside list comprehension (no for)`` () = FSharp """ module M -open System.Collections.Immutable -let v (b: ImmutableHashSet.Builder | null) = [ - match b with +let v (s: string | null) = [ + match s with | null -> () - | x -> yield x.ToImmutable() + | x -> yield x.Length ] """ |> withVersionAndCheckNulls ("preview", true)