Update evaluateCastCheck for exact types#7541
Conversation
The logic for joining nullability into a PossibleContents cone value did not previously preserve the exactness of the type in the cone value, causing assertion failures.
Exact references are known to point to exactly their heap type and not one of its subtypes. GUFA already analyzed exactness via the more general "cone" types that include an allowed subtyping depth. Exact types are equivalent to cone types of depth zero. Let GUFA take advantage of exactness information by normalizing cone depths to 0 whenever the cone type is exact.
Array2Struct has to update the types of every expression that interacts with and produces a reference to the optimized array type. It previously did this by separately checking whether a nullable or non-nullable reference to the array was a subtype of the expression's type. Simplify this logic by doing only a single check that considers only the heap types of the references. Also remove some unnecessary variables in which various reference types were cached since it is extremely cheap to materialize a reference type now. These simplifications will also make it easier to update the pass to handle exact reference types once `array.new` instructions are typed as exact.
Additionally use finality of heap types to infer that a value being cast must have exactly its static type, which allows us to infer that more casts to exact types will be successful. Add exhaustive unit tests for `evaluateCastCheck` for various interesting heap type relationships.
| auto castHeapType = castType.getHeapType(); | ||
| auto refIsHeapSubType = HeapType::isSubType(refHeapType, castHeapType); | ||
|
|
||
| // Check whether a value of type `a` is known to be compatible with type `b` |
There was a problem hiding this comment.
what does "compatible" here mean, if not "is a subtype" - ?
There was a problem hiding this comment.
I will simplify this to "Check whether a value of type a is known to have type b". The only reason not to use subtype is because we're relating a value and a type, not two types.
| } | ||
| // Ignore nullability. | ||
| return Type::isSubType(a.with(NonNullable), b.with(NonNullable)); | ||
| }; |
There was a problem hiding this comment.
I'm still not following this comment+code. If a is a user-defined heap type, why does it matter if it is open or not?
E.g. I would expect isHeapSubtype(child, parent) to return true regardless of the openness of child, since child "also has type" parent. Or do I not understand what "also has type" means?
There was a problem hiding this comment.
The "heap type" being checked here includes exactness, which is why we are actually comparing types and selectively ignoring nullability.
We're not just comparing the static type of the cast input with the cast target type. Instead, we're comparing the most specific safe approximation of the dynamic type of the cast input we can get with the cast target. If we know the value has dynamic heap type $foo, and $foo is final and cannot have subtypes, then we also know that the value must have dynamic heap type (exact $foo) because it cannot have any other type and still be a $foo.
There was a problem hiding this comment.
Sorry, what is a "dynamic type"?
There was a problem hiding this comment.
The type of the value at runtime.
There was a problem hiding this comment.
Thanks, now I can follow the code here.
I wrote a comment suggestion above.
| // Check whether a value of type `a` is known to also have type `b`, assuming | ||
| // it is non-null. | ||
| auto isHeapSubtype = [](Type a, Type b) { | ||
| // TODO: Use information from a subtypes analysis, if available. |
There was a problem hiding this comment.
| // TODO: Use information from a subtypes analysis, if available. | |
| // Infer if a must be exact: if it has no subtypes, then it must be. | |
| // TODO: Use information from a subtypes analysis, if available. |
| } | ||
| // Ignore nullability. | ||
| return Type::isSubType(a.with(NonNullable), b.with(NonNullable)); | ||
| }; |
There was a problem hiding this comment.
Thanks, now I can follow the code here.
I wrote a comment suggestion above.
| // here, and anyhow we will be removing the reference during Struct2Local, | ||
| // later.) | ||
| curr->type = nonNullStruct; | ||
| curr->type = Type(structType, NonNullable); |
There was a problem hiding this comment.
Was this diff not part of another PR?
There was a problem hiding this comment.
Yes. That PR was merged, so now it appears here :(
2 participants
Additionally use finality of heap types to infer that a value being cast
must have exactly its static type, which allows us to infer that more
casts to exact types will be successful.
Add exhaustive unit tests for
evaluateCastCheckfor variousinteresting heap type relationships.