-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Treat Refinements more like AndTypes #12317
Conversation
Fixes scala#12306 When comparing with an AndType on the left and a RefinedType on the right, decompose the right hand side into an AndType of types that have a single refinement each. This makes sure we lose the least information in the necessary subsequent `either` call.
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.
I've thought about this, tried to find cases where it would go wrong and I think everything looks good.
case tp: AndType => true | ||
case OrType(tp1, tp2) => containsAnd(tp1) || containsAnd(tp2) | ||
case tp: TypeParamRef => containsAnd(bounds(tp).hi) | ||
case tp: TypeRef => containsAnd(tp.info.hiBound) || tp.symbol.onGadtBounds(gbounds => containsAnd(gbounds.hi)) |
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.
Ah, interesting. We always compare with GADT bounds in frozen constraint, but that doesn't end up mattering, right? That is: do I understand correctly that even if we're not inferring constraints, we need to break down structural types on the RHS to properly check if there's subtyping between the left- and right-hand sides?
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.
I think the same reasoning applies: If we end up comparing GADT constraints we need necessaryEither, and that means we need to decompose the RHS as much as possible so that necesssaryEither is sound.
Fixes #12306
When comparing with an AndType on the left and a RefinedType on the right,
decompose the right hand side into an AndType of types that have a single
refinement each. This makes sure we lose the least possible information in the
necessary subsequent
either
call.This is a much better fix than #12316.