-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix i11545 #12087
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
Fix i11545 #12087
Conversation
If we do decide to merge this, I'll open a new issue for investigating in detail what is the exact problem here, and how to fix it well. |
// if base is a disjunction, this might have come from a tp1 type that | ||
// expands to a match type. In this case, we should try to reduce the type | ||
// and compare the redux. This is done in fourthTry | ||
def tryBaseType(cls2: Symbol): Boolean = { |
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.
This looks like it will overshoot. tryBaseType
is an essential element for comparisons. Simply ignoring this step under GADT constraint inference is too drastic. I would expect instead a necessaryEither somewhere in the logic.
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.
We only avoid tryBaseType
if the LHS is an AndType. In that case, using the base type can end up performing a comparison that is too approximate to be useful for GADTs (see neg/i11545a.scala, added in this PR). I have ideas for other fixes, we can discuss them during today's lab meeting.
Duplicate of #15175 |
This is more of a makeshift fix than anything else. If we care about plugging the soundness hole for 3.0.0, then we should merge it. Otherwise, I'll have to spend a bit more time fiddling with TypeComparer in order to properly fix the issue.