Skip to content

Commit a15e243

Browse files
CopilotT-Gro
andauthored
Fix SRTP nullness constraint resolution for types imported from older assemblies (#18785)
* Fix SRTP nullness constraint resolution for AmbivalentToNull types - Modified SolveNullnessSupportsNull in ConstraintSolver.fs to use legacy F# nullness rules - AmbivalentToNull types now only satisfy 'T : null if TypeNullIsExtraValue returns true - Added comprehensive tests for SRTP nullness constraint resolution issues #18390 and #18344 - Tests cover different language versions and nullness settings matrix --------- Co-authored-by: Tomas Grosup <[email protected]>
1 parent ccc636d commit a15e243

File tree

7 files changed

+75
-20
lines changed

7 files changed

+75
-20
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
* Fix active pattern typechecking regression. ([Issue #18638](https://github.com/dotnet/fsharp/issues/18638), [PR #18642](https://github.com/dotnet/fsharp/pull/18642))
1919
* Fix nullness warnings when casting non-nullable values to `IEquatable<T>` to match C# behavior. ([Issue #18759](https://github.com/dotnet/fsharp/issues/18759))
2020
* Fix IsByRefLikeAttribute types being incorrectly suppressed in completion lists. Types like `Span<T>` and `ReadOnlySpan<T>` now appear correctly in IntelliSense.
21+
22+
* Fix SRTP nullness constraint resolution for types imported from older assemblies. AmbivalentToNull types now use legacy F# nullness rules instead of always satisfying `'T : null` constraints. ([Issue #18390](https://github.com/dotnet/fsharp/issues/18390), [Issue #18344](https://github.com/dotnet/fsharp/issues/18344))
2123
* Fix Show XML doc for enum fields in external metadata ([Issue #17939](https://github.com/dotnet/fsharp/issues/17939#issuecomment-3137410105), [PR #18800](https://github.com/dotnet/fsharp/pull/18800))
2224

2325
### Changed

src/Compiler/Checking/ConstraintSolver.fs

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,7 +1024,8 @@ and SolveTypMeetsTyparConstraints (csenv: ConstraintSolverEnv) ndeep m2 trace ty
10241024
| TyparConstraint.SimpleChoice(tys, m2) -> SolveTypeChoice csenv ndeep m2 trace ty tys
10251025
| TyparConstraint.CoercesTo(ty2, m2) -> SolveTypeSubsumesTypeKeepAbbrevs csenv ndeep m2 trace None ty2 ty
10261026
| TyparConstraint.MayResolveMember(traitInfo, m2) ->
1027-
SolveMemberConstraint csenv false PermitWeakResolution.No ndeep m2 trace traitInfo |> OperationResult.ignore
1027+
SolveMemberConstraint csenv false PermitWeakResolution.No ndeep m2 trace traitInfo
1028+
|> OperationResult.ignore
10281029
}
10291030

10301031
and shouldWarnUselessNullCheck (csenv:ConstraintSolverEnv) =
@@ -2659,18 +2660,28 @@ and SolveTypeUseSupportsNull (csenv: ConstraintSolverEnv) ndeep m2 trace ty =
26592660
if not g.checkNullness && not (TypeNullIsExtraValue g m ty) then
26602661
return! ErrorD (ConstraintSolverError(FSComp.SR.csTypeDoesNotHaveNull(NicePrint.minimalStringOfType denv ty), m, m2))
26612662
else
2662-
if TypeNullIsExtraValue g m ty then
2663-
()
2664-
elif isNullableTy g ty then
2665-
return! ErrorD (ConstraintSolverError(FSComp.SR.csNullableTypeDoesNotHaveNull(NicePrint.minimalStringOfType denv ty), m, m2))
2666-
else
2667-
match tryDestTyparTy g ty with
2668-
| ValueSome tp ->
2669-
do! AddConstraint csenv ndeep m2 trace tp (TyparConstraint.SupportsNull m)
2670-
| ValueNone ->
2671-
return! ErrorD (ConstraintSolverError(FSComp.SR.csTypeDoesNotHaveNull(NicePrint.minimalStringOfType denv ty), m, m2))
2663+
// Use legacy F# nullness rules when langFeatureNullness is disabled
2664+
do! SolveLegacyTypeUseSupportsNullLiteral csenv ndeep m2 trace ty
26722665
}
26732666

2667+
// Common logic for legacy F# nullness rules - used for both non-langFeatureNullness path and AmbivalentToNull types
2668+
and SolveLegacyTypeUseSupportsNullLiteral (csenv: ConstraintSolverEnv) ndeep m2 (trace: OptionalTrace) ty =
2669+
trackErrors {
2670+
let g = csenv.g
2671+
let m = csenv.m
2672+
let denv = csenv.DisplayEnv
2673+
if TypeNullIsExtraValue g m ty then
2674+
()
2675+
elif isNullableTy g ty then
2676+
return! ErrorD (ConstraintSolverError(FSComp.SR.csNullableTypeDoesNotHaveNull(NicePrint.minimalStringOfType denv ty), m, m2))
2677+
else
2678+
match tryDestTyparTy g ty with
2679+
| ValueSome tp ->
2680+
do! AddConstraint csenv ndeep m2 trace tp (TyparConstraint.SupportsNull m)
2681+
| ValueNone ->
2682+
return! ErrorD (ConstraintSolverError(FSComp.SR.csTypeDoesNotHaveNull(NicePrint.minimalStringOfType denv ty), m, m2))
2683+
}
2684+
26742685
and SolveNullnessSupportsNull (csenv: ConstraintSolverEnv) ndeep m2 (trace: OptionalTrace) ty nullness =
26752686
trackErrors {
26762687
let g = csenv.g
@@ -2684,7 +2695,9 @@ and SolveNullnessSupportsNull (csenv: ConstraintSolverEnv) ndeep m2 (trace: Opti
26842695
trace.Exec (fun () -> nv.Set KnownWithNull) (fun () -> nv.Unset())
26852696
| Nullness.Known n1 ->
26862697
match n1 with
2687-
| NullnessInfo.AmbivalentToNull -> ()
2698+
| NullnessInfo.AmbivalentToNull ->
2699+
// For AmbivalentToNull types (imported from older assemblies), use legacy F# nullness rules
2700+
do! SolveLegacyTypeUseSupportsNullLiteral csenv ndeep m2 trace ty
26882701
| NullnessInfo.WithNull -> ()
26892702
| NullnessInfo.WithoutNull ->
26902703
if g.checkNullness then

src/Compiler/Symbols/SymbolHelpers.fs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -407,9 +407,9 @@ module internal SymbolHelpers =
407407

408408
member x.Equals(item1, item2) =
409409
match item1,item2 with
410-
| null,null -> true
411-
| null,_ | _,null -> false
412-
| item1,item2 ->
410+
| Null,Null -> true
411+
| Null,_ | _,Null -> false
412+
| NonNull item1,NonNull item2 ->
413413
// This may explore assemblies that are not in the reference set.
414414
// In this case just bail out and assume items are not equal
415415
protectAssemblyExploration false (fun () ->

src/Compiler/TypedTree/TypedTree.fs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4453,9 +4453,9 @@ type TType =
44534453
scope.QualifiedName
44544454

44554455
[<DebuggerBrowsable(DebuggerBrowsableState.Never)>]
4456-
member x.DebugText = x.ToString()
4456+
member x.DebugText = x.LimitedToString(4)
44574457

4458-
override x.ToString() =
4458+
member x.LimitedToString(maxDepth:int) =
44594459
match x with
44604460
| TType_forall (_tps, ty) -> "forall ... " + ty.ToString()
44614461
| TType_app (tcref, tinst, nullness) -> tcref.DisplayName + (match tinst with [] -> "" | tys -> "<" + String.concat "," (List.map string tys) + ">") + nullness.ToString()
@@ -4474,9 +4474,12 @@ type TType =
44744474
| TType_var (tp, _) ->
44754475
match tp.Solution with
44764476
| None -> tp.DisplayName
4477-
| Some _ -> tp.DisplayName + " (solved)"
4477+
| Some t -> tp.DisplayName + $" (solved: {if maxDepth < 0 then Boolean.TrueString else t.LimitedToString(maxDepth-1)})"
44784478
| TType_measure ms -> ms.ToString()
44794479

4480+
override x.ToString() = x.LimitedToString(4)
4481+
4482+
44804483
type TypeInst = TType list
44814484

44824485
type TTypes = TType list

tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@
349349
<Compile Include="FSharpChecker\TransparentCompiler.fs" />
350350
<Compile Include="FSharpChecker\SymbolUse.fs" />
351351
<Compile Include="FSharpChecker\FindReferences.fs" />
352-
<Compile Include="Attributes\AttributeCtorSetPropAccess.fs"/>
352+
<Compile Include="Attributes\AttributeCtorSetPropAccess.fs" />
353353
</ItemGroup>
354354

355355
<ItemGroup>

tests/FSharp.Compiler.Private.Scripting.UnitTests/FSharpScriptTests.fs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,42 @@ printfn "{@"%A"}" result
330330
Assert.Equal(1, (errors |> Seq.filter (fun error -> error.Message.Contains("FSharp.Really.Not.A.Package")) |> Seq.length))
331331
Assert.Equal(1, (errors |> Seq.filter (fun error -> error.Message.Contains("FSharp.Really.Not.Another.Package")) |> Seq.length))
332332

333+
[<Fact>]
334+
member _.``FsharpPlus - report errors``() =
335+
let code = """
336+
#i "nuget:https://api.nuget.org/v3/index.json"
337+
#r "nuget: FSharpPlus, 1.6.1"
338+
339+
open FSharpPlus
340+
open FSharpPlus.Data
341+
342+
let printTable x =
343+
let lines (lst: 'Record list) =
344+
let fields = Reflection.FSharpType.GetRecordFields typeof<'Record>
345+
let headers = fields |> Seq.map _.Name
346+
let asList (x:'record) = fields |> Seq.map (fun field -> string (Reflection.FSharpValue.GetRecordField(x, field)))
347+
let rows = Seq.map asList lst
348+
let table = seq { yield headers; yield! rows }
349+
let maxs = table |> (Seq.traverse ZipList >> ZipList.run) |>> Seq.map length |>> maxBy id
350+
let rowSep = String.replicate (sum maxs + length maxs - 1) "-"
351+
let fill (i, s) = s + String.replicate (i - length s) " "
352+
let printRow r = "|" + (r |> zip maxs |>> fill |> intercalate "|") + "|"
353+
seq {
354+
yield "." + rowSep + "."
355+
yield printRow headers
356+
yield "|" + rowSep + "|"
357+
yield! (rows |>> printRow)
358+
yield "'" + rowSep + "'" }
359+
x |> lines |> iter (printfn "%s")
360+
x |> List.length
361+
362+
printTable [{|Age = 15; Weight = 88; Name = "Blahboolahboogaloo"|}]
363+
"""
364+
use script = new FSharpScript(additionalArgs=[| |])
365+
let opt = script.Eval(code) |> getValue
366+
let value = opt.Value
367+
Assert.Equal(1, downcast value.ReflectionValue)
368+
333369
[<Fact>]
334370
member _.``ML - use assembly with ref dependencies``() =
335371
let code = """

tests/FSharp.Compiler.Service.Tests/ProjectAnalysisTests.fs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5805,7 +5805,8 @@ let checkContentAsScript content =
58055805
| FSharpCheckFileAnswer.Succeeded r -> r
58065806

58075807
[<Collection(nameof NotThreadSafeResourceCollection)>]
5808-
module ScriptClosureCacheUse =
5808+
module ScriptClosureCacheUse =
5809+
58095810
[<Fact>]
58105811
let ``References from #r nuget are included in script project options`` () =
58115812
let checkResults = checkContentAsScript """

0 commit comments

Comments
 (0)