Skip to content

pattern matcher still allows unsound case class reconstruction #7886

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

Open
scabug opened this issue Sep 30, 2013 · 10 comments
Open

pattern matcher still allows unsound case class reconstruction #7886

scabug opened this issue Sep 30, 2013 · 10 comments
Labels
fixed in Scala 3 This issue does not exist in the Scala 3 compiler (https://github.com/lampepfl/dotty/) patmat should not compile
Milestone

Comments

@scabug
Copy link

scabug commented Sep 30, 2013

Note that without the second parameter to Unravel, this is correctly failed in 2.10 (but not in 2.9.)

a.scala:11: error: type mismatch;
 found   : Contra[Nothing]
 required: Contra[Any]
    case Unravel(m) => g(m)
                         ^
one error found

But with it:

trait Covariant[+A]
trait Contra[-A] { def accept(p: A): Unit }
trait Invariant[A] extends Covariant[A] with Contra[A]

case class Unravel[A](m: Contra[A], msg: A)

object Test extends Covariant[Any] {
  def g(m: Contra[Any]): Unit = m accept 5
  def f(x: Any): Unit = x match {
    case Unravel(m, msg) => g(m)
    case _               =>
  }
  def main(args: Array[String]) {
    f(Unravel[String](new Contra[String] { def accept(x: String) = x.length }, ""))
  }
}
// java.lang.ClassCastException: java.lang.Integer cannot be cast to java.lang.String
//   at Test$$anon$1.accept(a.scala:18)
//   at Test$.g(a.scala:13)
//   at Test$.f(a.scala:15)
//   at Test$.main(a.scala:18)
//   at Test.main(a.scala)
@scabug
Copy link
Author

scabug commented Sep 30, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7886?orig=1
Reporter: @paulp

@scabug
Copy link
Author

scabug commented Sep 30, 2013

@paulp said:
scala/scala#3005

@scabug
Copy link
Author

scabug commented Nov 21, 2013

@retronym said:
I'm assuming we forgot to close this ticket after merging. Please open a new ticket if there were residuals I've missed.

@scabug
Copy link
Author

scabug commented Nov 29, 2013

@retronym said (edited on Nov 29, 2013 9:15:34 AM UTC):
Reopening. Special casing Any doesn't seem to be the right course of action.

git diff /Users/jason/code/scala/test/files/neg/t7886.scala
diff --git a/test/files/neg/t7886.scala b/test/files/neg/t7886.scala
index 55d80a0..1db8be9 100644
--- a/test/files/neg/t7886.scala
+++ b/test/files/neg/t7886.scala
@@ -2,11 +2,12 @@ trait Covariant[+A]
 trait Contra[-A] { def accept(p: A): Unit }
 trait Invariant[A] extends Covariant[A] with Contra[A]

-case class Unravel[A](m: Contra[A], msg: A)
+trait T
+case class Unravel[A](m: Contra[A], msg: A) extends T

 object Test extends Covariant[Any] {
   def g(m: Contra[Any]): Unit = m accept 5
-  def f(x: Any): Unit = x match {
+  def f(x: T): Unit = x match {
     case Unravel(m, msg) => g(m)
     case _               =>
   }
ticket/5900-2 ~/code/scala scalac-hash v2.11.0-M7 test/files/neg/t7886.scala && scala-hash v2.11.0-M7 Test
java.lang.ClassCastException: java.lang.Integer cannot be cast to java.lang.String
	at Test$$anon$1.accept(t7886.scala:15)

@scabug
Copy link
Author

scabug commented Nov 29, 2013

@retronym said:
Here's the best way I've found to look at this:

https://gist.github.com/retronym/7704153

In that example, I've added the parent type T to circumvent the bandaid fix for pt=Any

The pattern matching inference unsoundly casts Unravel[_] to Unravel[Any].

@scabug
Copy link
Author

scabug commented Feb 12, 2014

@retronym said:
The residual test is added in scala/scala#3514 which also reverts the fix for the originally reported case, as it was responsible for the regression noted in the comments of #5900

@scabug
Copy link
Author

scabug commented May 5, 2014

@retronym said:
Another example from #8563.

object Test extends App {
  sealed trait Node[+A]
  case class L[C,D](f: C => D) extends Node[C => D]
 
  def test[A,B](n: Node[A => B]): A => B = n match {
    case l: L[c,d] => l.f
  }
 
  println {
    test(new L[Int,Int](identity) with Node[Nothing]: Node[Int => String])(3): String // java.lang.Integer cannot be cast to java.lang.String
  }
}

@scabug
Copy link
Author

scabug commented May 5, 2014

@retronym said:
Maybe that's a different bug, as it also is unsound with a manually expaneded unapply.

@scabug
Copy link
Author

scabug commented May 16, 2014

@adriaanm said:
See also #8241

@scabug
Copy link
Author

scabug commented Jun 22, 2016

@retronym said:
Another example from Noel Welsh + @djspiewak

  {
    sealed trait Foo[A] {
      def run: A =
        this match {
          case Map(elt, f) => f("Can I have some type safety please?x")
        }
    }
    final case class Map[A,B](elt: A, f: A => B) extends Foo[B]
    // expand the case class and a type error is issued, as per examples above.
    // final class Map[A,B](val elt: A, val f: A => B) extends Foo[B]
    // object Map {
    //  def unapply[A, B](m: Map[A, B]): Option[(A, A => B)] = Some((m.elt, m.f))
    //}
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed in Scala 3 This issue does not exist in the Scala 3 compiler (https://github.com/lampepfl/dotty/) patmat should not compile
Projects
None yet
Development

No branches or pull requests

3 participants