Skip to content

Commit 3c30609

Browse files
authored
Type subsumption cache: handle unsolved type vars (#19040)
1 parent 956bfd3 commit 3c30609

File tree

8 files changed

+180
-64
lines changed

8 files changed

+180
-64
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
* Type relations cache: handle potentially "infinite" types ([PR #19010](https://github.com/dotnet/fsharp/pull/19010))
1010
* Disallow recursive structs with lifted type parameters ([Issue #18993](https://github.com/dotnet/fsharp/issues/18993), [PR #19031](https://github.com/dotnet/fsharp/pull/19031))
1111
* Fix units-of-measure changes not invalidating incremental builds. ([Issue #19049](https://github.com/dotnet/fsharp/issues/19049))
12+
* 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))
1213

1314
### Added
1415

src/Compiler/Checking/TypeRelations.fs

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,14 @@ type CanCoerce =
2929
type TTypeCacheKey =
3030
| TTypeCacheKey of TypeStructure * TypeStructure * CanCoerce
3131
static member TryGetFromStrippedTypes(ty1, ty2, canCoerce) =
32-
let t1, t2 = getTypeStructure ty1, getTypeStructure ty2
33-
if t1.IsPossiblyInfinite || t2.IsPossiblyInfinite then
34-
ValueNone
35-
else
36-
ValueSome (TTypeCacheKey(t1, t2, canCoerce))
32+
let tryGetTypeStructure ty =
33+
match ty with
34+
| TType_app _ ->
35+
tryGetTypeStructureOfStrippedType ty
36+
| _ -> ValueNone
37+
38+
(tryGetTypeStructure ty1, tryGetTypeStructure ty2)
39+
||> ValueOption.map2(fun t1 t2 -> TTypeCacheKey(t1, t2, canCoerce))
3740

3841
let getTypeSubsumptionCache =
3942
let factory (g: TcGlobals) =
@@ -137,10 +140,6 @@ let rec TypeFeasiblySubsumesType ndeep (g: TcGlobals) (amap: ImportMap) m (ty1:
137140

138141
let checkSubsumes ty1 ty2 =
139142
match ty1, ty2 with
140-
| TType_measure _, TType_measure _
141-
| TType_var _, _ | _, TType_var _ ->
142-
true
143-
144143
| TType_app (tc1, l1, _), TType_app (tc2, l2, _) when tyconRefEq g tc1 tc2 ->
145144
List.lengthsEqAndForall2 (TypesFeasiblyEquiv ndeep g amap m) l1 l2
146145

@@ -160,13 +159,17 @@ let rec TypeFeasiblySubsumesType ndeep (g: TcGlobals) (amap: ImportMap) m (ty1:
160159
// See if any interface in type hierarchy of ty2 is a supertype of ty1
161160
List.exists (TypeFeasiblySubsumesType (ndeep + 1) g amap m ty1 NoCoerce) interfaces
162161

163-
if g.langVersion.SupportsFeature LanguageFeature.UseTypeSubsumptionCache then
162+
match ty1, ty2 with
163+
| TType_measure _, TType_measure _
164+
| TType_var _, _ | _, TType_var _ ->
165+
true
166+
167+
| _ when g.langVersion.SupportsFeature LanguageFeature.UseTypeSubsumptionCache ->
164168
match TTypeCacheKey.TryGetFromStrippedTypes(ty1, ty2, canCoerce) with
165169
| ValueSome key ->
166170
(getTypeSubsumptionCache g).GetOrAdd(key, fun _ -> checkSubsumes ty1 ty2)
167171
| _ -> checkSubsumes ty1 ty2
168-
else
169-
checkSubsumes ty1 ty2
172+
| _ -> checkSubsumes ty1 ty2
170173

171174
and TypeFeasiblySubsumesTypeWithSupertypeCheck g amap m ndeep ty1 ty2 =
172175
match GetSuperTypeOfType g amap m ty2 with

src/Compiler/Utilities/TypeHashing.fs

Lines changed: 100 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
module internal Internal.Utilities.TypeHashing
22

33
open Internal.Utilities.Rational
4-
open Internal.Utilities.Library
54
open FSharp.Compiler.AbstractIL.IL
65
open FSharp.Compiler.Syntax
76
open FSharp.Compiler.TcGlobals
87
open FSharp.Compiler.Text
98
open FSharp.Compiler.TypedTree
109
open FSharp.Compiler.TypedTreeBasics
1110
open FSharp.Compiler.TypedTreeOps
12-
open System.Collections.Immutable
1311

1412
type ObserverVisibility =
1513
| PublicOnly
@@ -126,6 +124,7 @@ module HashAccessibility =
126124
| _ -> true
127125

128126
module rec HashTypes =
127+
129128
/// Hash a reference to a type
130129
let hashTyconRef tcref = hashTyconRefImpl tcref
131130

@@ -378,110 +377,159 @@ module HashTastMemberOrVals =
378377
/// * Uses per-compilation stamps (entities, typars, anon records, measures).
379378
/// * Emits shape for union cases (declaring type stamp + case name), tuple structness,
380379
/// function arrows, forall binders, nullness, measures, generic arguments.
381-
/// * Unknown/variable nullness => NeverEqual token to force inequality (avoid unsound hits).
380+
/// * Does not include type constraints.
382381
///
383382
/// Non-goals:
384383
/// * Cross-compilation stability.
385384
/// * Perfect canonicalisation or alpha-equivalence collapsing.
386385
///
387386
/// </summary>
388387
module StructuralUtilities =
389-
[<Struct; CustomEquality; NoComparison>]
390-
type NeverEqual =
391-
struct
392-
interface System.IEquatable<NeverEqual> with
393-
member _.Equals _ = false
394-
395-
override _.Equals _ = false
396-
override _.GetHashCode() = 0
397-
end
398-
399-
static member Singleton = NeverEqual()
388+
open Internal.Utilities.Library.Extras
400389

401390
[<Struct; NoComparison; RequireQualifiedAccess>]
402391
type TypeToken =
403392
| Stamp of stamp: Stamp
404393
| UCase of name: string
405394
| Nullness of nullness: NullnessInfo
395+
| NullnessUnsolved
406396
| TupInfo of b: bool
397+
| Forall of int
407398
| MeasureOne
408399
| MeasureRational of int * int
409-
| NeverEqual of never: NeverEqual
400+
| Solved of int
401+
| Unsolved of int
402+
| Rigid of int
410403

411404
type TypeStructure =
412-
| TypeStructure of TypeToken[]
413-
| PossiblyInfinite of never: NeverEqual
405+
| Stable of TypeToken[]
406+
| Unstable of TypeToken[]
407+
| PossiblyInfinite
408+
409+
type private EmitContext =
410+
{
411+
typarMap: System.Collections.Generic.Dictionary<Stamp, int>
412+
emitNullness: bool
413+
mutable stable: bool
414+
}
414415

415-
let inline toNullnessToken (n: Nullness) =
416-
match n.TryEvaluate() with
417-
| ValueSome k -> TypeToken.Nullness k
418-
| _ -> TypeToken.NeverEqual NeverEqual.Singleton
416+
let private emitNullness env (n: Nullness) =
417+
seq {
418+
if env.emitNullness then
419+
env.stable <- false //
420+
421+
match n.TryEvaluate() with
422+
| ValueSome k -> TypeToken.Nullness k
423+
| ValueNone -> TypeToken.NullnessUnsolved
424+
}
419425

420-
let rec private accumulateMeasure (m: Measure) =
426+
let rec private emitMeasure (m: Measure) =
421427
seq {
422428
match m with
423429
| Measure.Var mv -> TypeToken.Stamp mv.Stamp
424430
| Measure.Const(tcref, _) -> TypeToken.Stamp tcref.Stamp
425431
| Measure.Prod(m1, m2, _) ->
426-
yield! accumulateMeasure m1
427-
yield! accumulateMeasure m2
428-
| Measure.Inv m1 -> yield! accumulateMeasure m1
432+
yield! emitMeasure m1
433+
yield! emitMeasure m2
434+
| Measure.Inv m1 -> yield! emitMeasure m1
429435
| Measure.One _ -> TypeToken.MeasureOne
430436
| Measure.RationalPower(m1, r) ->
431-
yield! accumulateMeasure m1
437+
yield! emitMeasure m1
432438
TypeToken.MeasureRational(GetNumerator r, GetDenominator r)
433439
}
434440

435-
let rec private accumulateTType (ty: TType) =
441+
and private emitTType (env: EmitContext) (ty: TType) =
436442
seq {
437443
match ty with
438444
| TType_ucase(u, tinst) ->
439445
TypeToken.Stamp u.TyconRef.Stamp
440446
TypeToken.UCase u.CaseName
441447

442448
for arg in tinst do
443-
yield! accumulateTType arg
449+
yield! emitTType env arg
444450

445451
| TType_app(tcref, tinst, n) ->
446452
TypeToken.Stamp tcref.Stamp
447-
toNullnessToken n
453+
yield! emitNullness env n
448454

449455
for arg in tinst do
450-
yield! accumulateTType arg
456+
yield! emitTType env arg
457+
451458
| TType_anon(info, tys) ->
452459
TypeToken.Stamp info.Stamp
453460

454461
for arg in tys do
455-
yield! accumulateTType arg
462+
yield! emitTType env arg
463+
456464
| TType_tuple(tupInfo, tys) ->
457465
TypeToken.TupInfo(evalTupInfoIsStruct tupInfo)
458466

459467
for arg in tys do
460-
yield! accumulateTType arg
468+
yield! emitTType env arg
469+
461470
| TType_forall(tps, tau) ->
462471
for tp in tps do
463-
TypeToken.Stamp tp.Stamp
472+
env.typarMap.[tp.Stamp] <- env.typarMap.Count
473+
474+
TypeToken.Forall tps.Length
475+
476+
yield! emitTType env tau
464477

465-
yield! accumulateTType tau
466478
| TType_fun(d, r, n) ->
467-
yield! accumulateTType d
468-
yield! accumulateTType r
469-
toNullnessToken n
479+
yield! emitTType env d
480+
yield! emitTType env r
481+
yield! emitNullness env n
482+
470483
| TType_var(r, n) ->
471-
TypeToken.Stamp r.Stamp
472-
toNullnessToken n
473-
| TType_measure m -> yield! accumulateMeasure m
484+
yield! emitNullness env n
485+
486+
let typarId =
487+
match env.typarMap.TryGetValue r.Stamp with
488+
| true, idx -> idx
489+
| _ ->
490+
let idx = env.typarMap.Count
491+
env.typarMap.[r.Stamp] <- idx
492+
idx
493+
494+
// Solved may become unsolved, in case of Trace.Undo.
495+
env.stable <- false
496+
497+
match r.Solution with
498+
| Some ty -> yield! emitTType env ty
499+
| None ->
500+
if r.Rigidity = TyparRigidity.Rigid then
501+
TypeToken.Rigid typarId
502+
else
503+
TypeToken.Unsolved typarId
504+
505+
| TType_measure m -> yield! emitMeasure m
474506
}
475507

476-
// If the sequence got too long, just drop it, we could be dealing with an infinite type.
477-
let private toTypeStructure tokens =
478-
let tokens = tokens |> Seq.truncate 256 |> Array.ofSeq
479-
480-
if tokens.Length = 256 then
481-
PossiblyInfinite NeverEqual.Singleton
482-
else
483-
TypeStructure tokens
484-
485-
/// Get the full structure of a type as a sequence of tokens, suitable for equality
486-
let getTypeStructure =
487-
Extras.WeakMap.getOrCreate (fun ty -> accumulateTType ty |> toTypeStructure)
508+
let private getTypeStructureOfStrippedType (ty: TType) =
509+
510+
let env =
511+
{
512+
typarMap = System.Collections.Generic.Dictionary<Stamp, int>()
513+
emitNullness = false
514+
stable = true
515+
}
516+
517+
let tokens = emitTType env ty |> Seq.truncate 256 |> Seq.toArray
518+
519+
// If the sequence got too long, just drop it, we could be dealing with an infinite type.
520+
if tokens.Length = 256 then PossiblyInfinite
521+
elif not env.stable then Unstable tokens
522+
else Stable tokens
523+
524+
// Speed up repeated calls by memoizing results for types that yield a stable structure.
525+
let private memoize =
526+
WeakMap.cacheConditionally
527+
(function
528+
| Stable _ -> true
529+
| _ -> false)
530+
getTypeStructureOfStrippedType
531+
532+
let tryGetTypeStructureOfStrippedType ty =
533+
match memoize ty with
534+
| PossiblyInfinite -> ValueNone
535+
| ts -> ValueSome ts

src/Compiler/Utilities/lib.fs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,3 +473,15 @@ module WeakMap =
473473
// Cached factory to avoid allocating a new lambda per lookup.
474474
let factory = ConditionalWeakTable.CreateValueCallback(fun k -> valueFactory k)
475475
fun (key: 'Key when 'Key: not null) -> table.GetValue(key, factory)
476+
477+
/// Like getOrCreate, but only cache the value if it satisfies the given predicate.
478+
let cacheConditionally shouldCache valueFactory =
479+
let table = ConditionalWeakTable<_, _>()
480+
fun (key: 'Key when 'Key: not null) ->
481+
match table.TryGetValue key with
482+
| true, value -> value
483+
| false, _ ->
484+
let value = valueFactory key
485+
if shouldCache value then
486+
try table.Add(key, value) with _ -> ()
487+
value

src/Compiler/Utilities/lib.fsi

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,3 +307,8 @@ module internal WeakMap =
307307
val internal getOrCreate:
308308
valueFactory: ('Key -> 'Value) -> ('Key -> 'Value)
309309
when 'Key: not struct and 'Key: not null and 'Value: not struct
310+
311+
/// Like getOrCreate, but only cache the value if it satisfies the given predicate.
312+
val cacheConditionally:
313+
shouldCache: ('Value -> bool) -> valueFactory: ('Key -> 'Value) -> ('Key -> 'Value)
314+
when 'Key: not struct and 'Key: not null and 'Value: not struct

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,7 @@
277277
<Compile Include="Interop\Literals.fs" />
278278
<Compile Include="Scripting\Interactive.fs" />
279279
<Compile Include="Scripting\TypeCheckOnlyTests.fs" />
280+
<Compile Include="TypeChecks\TypeRelations.fs" />
280281
<Compile Include="TypeChecks\SeqTypeCheckTests.fs" />
281282
<Compile Include="TypeChecks\CheckDeclarationsTests.fs" />
282283
<Compile Include="TypeChecks\Graph\Utils.fs" />
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
module MyModule
2+
3+
type IFoo<'T when 'T :> IFoo<'T>> =
4+
abstract member Bar: other:'T -> unit
5+
6+
[<AbstractClass>]
7+
type FooBase() =
8+
9+
interface IFoo<FooBase> with
10+
member this.Bar (other: FooBase) = ()
11+
12+
[<Sealed>]
13+
type FooDerived<'T>() =
14+
inherit FooBase()
15+
16+
interface IFoo<FooDerived<'T>> with
17+
member this.Bar other = ()
18+
19+
type IFooContainer<'T> =
20+
abstract member Foo: FooDerived<'T>
21+
22+
let inline bar<'a when 'a :> IFoo<'a>> (x: 'a) (y: 'a) = x.Bar y
23+
let inline takeSame<'a> (x: 'a) (y: 'a) = ()
24+
25+
// Successfully compiles under .NET 9 + F# 9
26+
// Error under .NET 10 + F# 10: Program.fs(26,13): Error FS0193 : The type 'FooDerived<'TId>' does not match the type 'FooBase'
27+
let callBar_NewlyBroken (foo1: IFooContainer<'TId>) (foo2: IFooContainer<'TId>) =
28+
bar foo1.Foo foo2.Foo
29+
30+
// Successfully compiles under both versions
31+
let callBar (foo1: IFooContainer<'TId>) (foo2: IFooContainer<'TId>) =
32+
let id1 = foo1.Foo
33+
let id2 = foo2.Foo
34+
bar id1 id2
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
module TypeChecks.TypeRelations
2+
3+
open Xunit
4+
open FSharp.Test.Compiler
5+
open FSharp.Test
6+
7+
[<Theory; FileInlineData("CrgpLibrary.fs")>]
8+
let ``Unsolved type variables are not cached`` compilation =
9+
compilation
10+
|> getCompilation
11+
|> typecheck
12+
|> shouldSucceed

0 commit comments

Comments
 (0)