[CS] Don't leave key path with holes unsolved #31829
Conversation
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
lib/Sema/CSSimplify.cpp
Outdated
There was a problem hiding this comment.
I think it was sufficient originally to stop when for type variable to be marked as a hole but the meaning of this option has been changed to indicate that it could be a hole. Instead of just returning here we should wait with short-circuiting of key path application until at least some of the components have been determined to be a hole. WDYT?
There was a problem hiding this comment.
@xedin That was my initial thought too, unfortunately types can currently back-propagate into the overload's type var, e.g for something like:
let kp: KeyPath<String, String> = \.bloop
Even though we record a fix for bloop being a missing member, and mark the type variable as being able to bind to a hole, the solver would rather bind it to String from the context. We probably need some way to mark the overload itself as missing rather than just not recording it in these cases, what do you think?
Fortunately for now at least I believe overload type variables aren't marked as potential holes unless they're erroneous in some way, so I believe this logic should work for now without prematurely short-circuiting. I can drop in a FIXME to clean this up.
There was a problem hiding this comment.
That's actually correct behavior, we'd want to have as few holes as possible but it's unfortunate that it interferes with this. I think you should drop a TODO here and leave it for now.
We should re-design how everything works around key path. I have already mentioned that to @LucianoPAlmeida in one of his PRs that key path should work just like closures do today - resolve key path itself only when components are resolved, key path type inference is only attempted once the last type variable representing a component gets bound to a type.
There was a problem hiding this comment.
I think, just like with untyped closure parameters, a better error message here would be cannot infer key path type from context; consider explicitly specifying a root type... But we could make it work separately.
There was a problem hiding this comment.
Yeah, I definitely agree it could be better! I've filed SR-12827 to track this.
|
Forgot to approve sorry :) |
We currently leave a key path constraint unsolved if one of its components hasn't yet had its overload resolved. However, for e.g a missing member component, the overload type variable will be bound to a hole and an overload will never be resolved. Tweak the logic to consider the key path constraint trivially solved if one of its components has been marked as a hole, which will allow the key path type itself to be marked as a hole. Resolves SR-12437 & SR-12823. Resolves rdar://62201037.
hamishknight
force-pushed
the
solved-holistically
branch
from
2f30651 to
aa0ad55
Compare
|
@swift-ci please smoke test |
3 participants
We currently leave a key path constraint unsolved if one of its components hasn't yet had its overload resolved. However, for e.g a missing member component, the overload type variable will be bound to a hole and an overload will never be resolved.
Tweak the logic to consider the key path constraint trivially solved if one of its components has been marked as a hole, which will allow the key path type itself to be marked as a hole.
Resolves SR-12437 & SR-12823.
Resolves rdar://62201037.