Skip to content
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

Type unsoundness: During GADT pattern matching, skolem for invariant type parameter is replaced by Any #7090

Closed
scabug opened this issue Feb 6, 2013 · 3 comments

Comments

@scabug
Copy link

scabug commented Feb 6, 2013

In the below code, pattern matching silently upcasts invariant type Exp, from Exp[Double] to Exp[Any] even though Exp is invariant.

case class Man[T]()
implicit def man[T] = Man[T]()

abstract class Exp[T:Man] {
  def mT = man[T]
}

case class Const[A:Man](x: A) extends Exp[A]

case class Node[A:Man, B:Man](base: Exp[A]) extends Exp[B] {
  def mA = man[A]
  def mB = man[B]
}

val m = Node[Int, String](Node[Double, Int](Const(3.14)))

val n = m match {
  case Node(Node(t)) => t
}
//At the REPL we get
//n: Exp[Any] = Const(3.14)

//scala> n.mT
//res0: Man[Any] = Man()

This example was shown to me by my colleague Cai Yufei; the above typing issue occurs when trying to write a type-correct map fusion in LMS-like scenarios - this explains the strange type for Node above.

The use of primitive types above is irrelevant, as shown below:

scala> val m = Node[java.io.File, StringBuffer](Node[String, java.io.File](Const("a string")))
m: Node[java.io.File,java.lang.StringBuffer] = Node(Node(Const(a string)))

scala> val n = m match {
     |   case Node(Node(t)) => t
     | }
n: Exp[Any] = Const(a string)
@scabug
Copy link
Author

scabug commented Feb 6, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7090?orig=1
Reporter: @Blaisorblade
Affected Versions: 2.9.2, 2.10.0
Duplicates #6680

@scabug
Copy link
Author

scabug commented Feb 6, 2013

@adriaanm said:
duplicate of SI-6680?

@scabug
Copy link
Author

scabug commented Feb 6, 2013

@Blaisorblade said:
AFAICS yes, sorry for the dup - at least as long as the fix works on both testcases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants