Skip to content

Fix #3333: adapt child instantiation #3475

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

Merged
merged 3 commits into from
Nov 20, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 24 additions & 7 deletions compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
Original file line number Diff line number Diff line change
Expand Up @@ -584,10 +584,9 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
// precondition: `tp1` should have the shape `path.Child`, thus `ThisType` is always covariant
val thisTypeMap = new TypeMap {
def apply(t: Type): Type = t match {
case tp @ ThisType(tref) if !tref.symbol.isStaticOwner && !tref.symbol.is(Module) =>
// TODO: stackoverflow here
// newTypeVar(TypeBounds.upper(mapOver(tp.underlying)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity: what was the original issue here? Was tp present somewhere in tp.underlying? If yes, then wouldn't simply first creating the TVar, memoising it in a map keyed under tp and mapping over tp.underlying be enough? I'm asking, as basing on this code I wrote this and I'm not sure if it is correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot exactly what's the problem, but it's related to ClassInfo#selfType. tp.underlying is supposed to be decreasing (I assume), so I think the case you mention will not occur. For the task you want to deal with, the memoization in your code is correct.

newTypeVar(TypeBounds.upper(mapOver(tref & tref.classSymbol.asClass.givenSelfType)))
case tp @ ThisType(tref) if !tref.symbol.isStaticOwner =>
if (tref.symbol.is(Module)) mapOver(tref)
else newTypeVar(TypeBounds.upper(tp.underlying))
case _ =>
mapOver(t)
}
Expand All @@ -596,7 +595,6 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
// replace type parameter references with bounds
val typeParamMap = new TypeMap {
def apply(t: Type): Type = t match {

case tp: TypeRef if tp.symbol.is(TypeParam) && tp.underlying.isInstanceOf[TypeBounds] =>
// See tests/patmat/gadt.scala tests/patmat/exhausting.scala tests/patmat/t9657.scala
val exposed =
Expand All @@ -611,13 +609,32 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
}
}

// replace uninstantiated type vars with WildcardType, check tests/patmat/3333.scala
val instUndetMap = new TypeMap {
def apply(t: Type): Type = t match {
case tvar: TypeVar if !tvar.isInstantiated => WildcardType(tvar.origin.underlying.bounds)
case _ => mapOver(t)
}
}

val force = new ForceDegree.Value(
tvar => !(ctx.typerState.constraint.entry(tvar.origin) eq tvar.origin.underlying),
minimizeAll = false
)

val tvars = tp1.typeParams.map { tparam => newTypeVar(tparam.paramInfo.bounds) }
val protoTp1 = thisTypeMap(tp1.appliedTo(tvars))

if (protoTp1 <:< tp2 && isFullyDefined(protoTp1, ForceDegree.noBottom)) protoTp1
if (protoTp1 <:< tp2) {
isFullyDefined(protoTp1, force)
instUndetMap(protoTp1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly how isFullyDefined behaves, then if it returns true, all TVars in protoTp1 should be already instantiated and instUndetMap will be a no-op. Would it make sense to only use instUndetMap if isFullyDefined returned false?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good catch, I've updated the code, thanks a lot.

}
else {
val protoTp2 = typeParamMap(tp2)
if (protoTp1 <:< protoTp2 && isFullyDefined(protoTp1 & protoTp2, ForceDegree.noBottom)) protoTp1
if (protoTp1 <:< protoTp2) {
isFullyDefined(AndType(protoTp1, protoTp2), force)
instUndetMap(protoTp1)
}
else {
debug.println(s"$protoTp1 <:< $protoTp2 = false")
NoType
Expand Down
1 change: 1 addition & 0 deletions tests/patmat/3333.check
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
10: Pattern Match Exhaustivity: _: IntNumber, NotNaN
14 changes: 14 additions & 0 deletions tests/patmat/3333.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
sealed trait IntegralNumber

object IntegralNumber {
object NaN extends IntegralNumber
object NotNaN extends IntegralNumber

sealed abstract class FiniteNumberImpl[N](val value: N) extends IntegralNumber
sealed class IntNumber(value: Int) extends FiniteNumberImpl[Int](value)

def test(o: IntegralNumber) = o match {
case NaN => -1
}

}
1 change: 1 addition & 0 deletions tests/patmat/3455.check
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
11: Pattern Match Exhaustivity: Decimal
14 changes: 14 additions & 0 deletions tests/patmat/3455.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
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"
}
}
9 changes: 9 additions & 0 deletions tests/patmat/3469.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
object O {
sealed trait Trait[+A] { type T }
case class CaseClass[+A](a: A) extends Trait[A] { type T = Nothing }

def id[TT, A](v: Trait[A] { type T = TT }): Trait[A] { type T = TT } =
v match {
case CaseClass(a) => CaseClass(a)
}
}