-
Notifications
You must be signed in to change notification settings - Fork 833
Type subsumption cache: handle unsolved type vars #19040
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
e3d1871
aec47e7
b4b951c
0939fba
ad681fa
5f86502
068b3b5
853ef55
b48fc74
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -379,17 +379,6 @@ module HashTastMemberOrVals = | |
| /// | ||
| /// </summary> | ||
| module StructuralUtilities = | ||
| [<Struct; CustomEquality; NoComparison>] | ||
| type NeverEqual = | ||
| struct | ||
| interface System.IEquatable<NeverEqual> with | ||
| member _.Equals _ = false | ||
|
|
||
| override _.Equals _ = false | ||
| override _.GetHashCode() = 0 | ||
| end | ||
|
|
||
| static member Singleton = NeverEqual() | ||
|
|
||
| [<Struct; NoComparison; RequireQualifiedAccess>] | ||
| type TypeToken = | ||
|
|
@@ -399,16 +388,17 @@ module StructuralUtilities = | |
| | TupInfo of b: bool | ||
| | MeasureOne | ||
| | MeasureRational of int * int | ||
| | NeverEqual of never: NeverEqual | ||
| | Unsolved | ||
|
|
||
| type TypeStructure = | ||
| | TypeStructure of TypeToken[] | ||
| | PossiblyInfinite of never: NeverEqual | ||
| | UnsolvedTypeStructure of TypeToken[] | ||
| | PossiblyInfinite | ||
|
|
||
| let inline toNullnessToken (n: Nullness) = | ||
| match n.TryEvaluate() with | ||
| | ValueSome k -> TypeToken.Nullness k | ||
| | _ -> TypeToken.NeverEqual NeverEqual.Singleton | ||
| | _ -> TypeToken.Unsolved | ||
|
|
||
| let rec private accumulateMeasure (m: Measure) = | ||
| seq { | ||
|
|
@@ -425,7 +415,14 @@ module StructuralUtilities = | |
| TypeToken.MeasureRational(GetNumerator r, GetDenominator r) | ||
| } | ||
|
|
||
| let rec private accumulateTType (ty: TType) = | ||
| let rec private accumulateTypar (typar: Typar) = | ||
| seq { | ||
| match typar.Solution with | ||
| | Some ty -> yield! accumulateTType ty | ||
| | None -> TypeToken.Unsolved | ||
| } | ||
|
|
||
| and private accumulateTType (ty: TType) = | ||
| seq { | ||
| match ty with | ||
| | TType_ucase(u, tinst) -> | ||
|
|
@@ -441,40 +438,55 @@ module StructuralUtilities = | |
|
|
||
| for arg in tinst do | ||
| yield! accumulateTType arg | ||
|
|
||
| | TType_anon(info, tys) -> | ||
| TypeToken.Stamp info.Stamp | ||
|
|
||
| for arg in tys do | ||
| yield! accumulateTType arg | ||
|
|
||
| | TType_tuple(tupInfo, tys) -> | ||
| TypeToken.TupInfo(evalTupInfoIsStruct tupInfo) | ||
|
|
||
| for arg in tys do | ||
| yield! accumulateTType arg | ||
|
|
||
| | TType_forall(tps, tau) -> | ||
| for tp in tps do | ||
| TypeToken.Stamp tp.Stamp | ||
| yield! accumulateTypar tp | ||
|
|
||
| yield! accumulateTType tau | ||
|
|
||
| | TType_fun(d, r, n) -> | ||
| yield! accumulateTType d | ||
| yield! accumulateTType r | ||
| toNullnessToken n | ||
|
|
||
| | TType_var(r, n) -> | ||
| TypeToken.Stamp r.Stamp | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that the stamp is gone, I feel we are missing something to differentiate constraints. Most likely types where constraints matter will be unsolved, so this will bypass the cache (via There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah this is missing constraints. I feel encoding them could cause this to explode in size. |
||
| toNullnessToken n | ||
| yield! accumulateTypar r | ||
|
|
||
| | TType_measure m -> yield! accumulateMeasure m | ||
| } | ||
|
|
||
| // If the sequence got too long, just drop it, we could be dealing with an infinite type. | ||
| let private toTypeStructure tokens = | ||
| let tokens = tokens |> Seq.truncate 256 |> Array.ofSeq | ||
| let private toTypeStructure (tokens: TypeToken seq) = | ||
| let tokens = tokens |> Seq.truncate 256 |> Seq.toArray | ||
|
|
||
| if tokens.Length = 256 then | ||
| PossiblyInfinite NeverEqual.Singleton | ||
| if Array.length tokens = 256 then | ||
| PossiblyInfinite | ||
| elif tokens |> Array.exists _.IsUnsolved then | ||
| UnsolvedTypeStructure tokens | ||
| else | ||
| TypeStructure tokens | ||
|
|
||
| /// Get the full structure of a type as a sequence of tokens, suitable for equality | ||
| let getTypeStructure = | ||
| Extras.WeakMap.getOrCreate (fun ty -> accumulateTType ty |> toTypeStructure) | ||
| let shouldCache = | ||
| function | ||
| | PossiblyInfinite | ||
| | UnsolvedTypeStructure _ -> false | ||
| | _ -> true | ||
|
|
||
| // Speed up repeated calls by caching results for types that yield a stable structure. | ||
| Extras.WeakMap.cacheConditionally shouldCache (fun ty -> accumulateTType ty |> toTypeStructure) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| module MyModule | ||
|
|
||
| type IFoo<'T when 'T :> IFoo<'T>> = | ||
| abstract member Bar: other:'T -> unit | ||
|
|
||
| [<AbstractClass>] | ||
| type FooBase() = | ||
|
|
||
| interface IFoo<FooBase> with | ||
| member this.Bar (other: FooBase) = () | ||
|
|
||
| [<Sealed>] | ||
| type FooDerived<'T>() = | ||
| inherit FooBase() | ||
|
|
||
| interface IFoo<FooDerived<'T>> with | ||
| member this.Bar other = () | ||
|
|
||
| type IFooContainer<'T> = | ||
| abstract member Foo: FooDerived<'T> | ||
|
|
||
| let inline bar<'a when 'a :> IFoo<'a>> (x: 'a) (y: 'a) = x.Bar y | ||
| let inline takeSame<'a> (x: 'a) (y: 'a) = () | ||
|
|
||
| // Successfully compiles under .NET 9 + F# 9 | ||
| // Error under .NET 10 + F# 10: Program.fs(26,13): Error FS0193 : The type 'FooDerived<'TId>' does not match the type 'FooBase' | ||
| let callBar_NewlyBroken (foo1: IFooContainer<'TId>) (foo2: IFooContainer<'TId>) = | ||
| bar foo1.Foo foo2.Foo | ||
|
|
||
| // Successfully compiles under both versions | ||
| let callBar (foo1: IFooContainer<'TId>) (foo2: IFooContainer<'TId>) = | ||
| let id1 = foo1.Foo | ||
| let id2 = foo2.Foo | ||
| bar id1 id2 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| module TypeChecks.TypeRelations | ||
|
|
||
| open Xunit | ||
| open FSharp.Test.Compiler | ||
| open FSharp.Test | ||
|
|
||
| [<Theory; FileInlineData("CrgpLibrary.fs")>] | ||
| let ``Unsolved type variables are not cached`` compilation = | ||
| compilation | ||
| |> getCompilation | ||
| |> typecheck | ||
| |> shouldSucceed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, the input going into the cache is already stripped, right?
i.e. we should not be getting long chains of solution pointers for something which is solved.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah since this is operating solely on stripped types, if we encounter a type var, it should never be a solved one. In theory.