[ConstraintSystem] Introduce a new type to represent a type hole #33658
Conversation
Instead of using `UnresolvedType` as a placeholder for a type hole, let's switch over to a dedicated "rich" `HoleType` which is capable of storing "originator" type - type variable or dependent member type which couldn't be resolved. This makes it easier for the solver to determine origins of a hole which helps to diagnose certain problems better. It also helps code completion to locate "expected type" of the context even when it couldn't be completely resolved.
…tion are `UnresolvedType`
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
|
Build failed |
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
|
Build failed |
|
Build failed |
| Type HoleType::get(ASTContext &ctx, OriginatorType originator) { | ||
| assert(originator); | ||
| RecursiveTypeProperties properties = RecursiveTypeProperties::HasTypeHole; | ||
| auto arena = getArena(properties); |
There was a problem hiding this comment.
I believe this is going to always allocate the type in the permanent arena, which is bad if it contains a pointer to a TypeVariableType. The recursive properties used to determine the arena should be the properties of the originator type, not the new properties of the hole type itself.
There was a problem hiding this comment.
I didn’t realize that, thank you, Slava!
There was a problem hiding this comment.
I need to make sure that types containing holes as well as hole types themselves are allocated in ConstraintSolver arena...
There was a problem hiding this comment.
Fixed! Every type which contains holes or is a HoleType itself is not going to be allocated in ConstraintSolver arena. @slavapestov WDYT?
…lver` arena Since `HoleType` directly as well as other types which could contain holes are bound to a lifetime of constraint system that created them, we need to make sure that such types are always allocated using `ConstraintSolver` arena instead of a permanent one.
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
@swift-ci please test Linux platform |
|
@swift-ci please test source compatibility |
|
Build failed |
|
@swift-ci please test Linux platform |
|
Outside of the solver, there are a number of places we assert that we aren't seeing types with type variables in them. Those should be extended to also assert about not seeing holes. |
|
I can do that for completeness and I have added an assert to |
|
Does this allow addressing the following TODO? |
|
@theblixguy Yes, that's one of the things which could be improved with "rich" hole types. |
|
@CodaFi Added more asserts to match what we have for type variables. |
|
@swift-ci please smoke test |
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
| @@ -126,7 +126,7 @@ namespace { | |||
| } // end anonymous namespace | |||
|
|
|||
| void Expr::setType(Type T) { | |||
| assert(!T || !T->hasTypeVariable()); | |||
| assert(!T || !T->hasTypeVariable() || !T->hasHole()); | |||
There was a problem hiding this comment.
What do you think about creating a isCheckedType() method that ensures that the type is not a UNCHECKED_TYPE node? That way we don't need to update every call site that does this: !T->hasTypeVariable().
There was a problem hiding this comment.
That makes sense to me but I think we can't do it at the moment unless UnresolvedType is excluded from that check because code completion is still replying on solutions with unresolved types applied to AST (we are currently working on fixing that)...
Instead of using
UnresolvedTypeas a placeholder for a type hole,let's switch over to a dedicated "rich"
HoleTypewhich is capableof storing "originator" type - type variable or dependent member
type which couldn't be resolved.
This makes it easier for the solver to determine origins of
a hole which helps to diagnose certain problems better. It also
helps code completion to locate "expected type" of the context
even when it couldn't be completely resolved.