-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #3938: adapt prefix to ease prefix inference #3939
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
Conversation
|
@odersky : we are doing prefix inference in exhaustivity check, this PR does some magic in order for inference to work smoothly. Could you have a look and see if it makes sense? |
| } | ||
|
|
||
| // Fix subtype checking for child instantiation, | ||
| // such that `Foo(Test.this.foo) <:< Foo(Foo.this)` |
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 could not correlate the claim Foo(Test.this.foo) <:< Foo(Foo.this) in i3939.scala with the code here. In i3938, foo is not a module, so how does the removeThisType affect it?
Also, note that if we do have a
object Foo
then we have already Foo.type <:< Foo.this and Foo.this <:< Foo.type by the rules for ThisType in TypeComparer's firstTry and secondTry methods. So I am not sure why the code would be necessesary.
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.
Thanks for the clarification. The correlation is that in the else branch, it throws away of the ThisType. For the module case, I'll try if I can throw it away.
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 just checked, the patch for module cannot be removed, as the subtyping for Foo.type <:< Foo.this only works if it's static owner:
case tp1: NamedType if cls2.is(Module) && cls2.eq(tp1.widen.typeSymbol) =>
cls2.isStaticOwner ||
isSubType(tp1.prefix, cls2.owner.thisType) ||
secondTry(tp1, tp2)The test case tests/patmat/3455.scala shows that we need the patch:
trait AxisCompanion {
sealed trait Format
object Format {
case object Decimal extends Format
case object Integer extends Format
}
}
object Axis extends AxisCompanion
class Axis {
import Axis._
def test( f: Format ) = f match {
case Format.Integer => "Int"
}
}
odersky
left a comment
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.
Thanks for explaining @liufengyun.
Fix #3938: adapt prefix to ease prefix inference