-
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? |
@@ -638,15 +640,31 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { | |||
} | |||
} | |||
|
|||
// 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"
}
}
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