Skip to content

Unsound GADT constraints with pattern alternatives #22805

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

Closed
lrytz opened this issue Mar 14, 2025 · 2 comments · Fixed by #22853
Closed

Unsound GADT constraints with pattern alternatives #22805

lrytz opened this issue Mar 14, 2025 · 2 comments · Fixed by #22853
Assignees
Labels
area:gadt itype:bug itype:soundness Soundness bug (it lets us compile code that crashes at runtime with a ClassCastException)
Milestone

Comments

@lrytz
Copy link
Member

lrytz commented Mar 14, 2025

Originally reported by @szeiger at scala/bug#13090

scala> sealed trait A[T] { def x: T }
     | final case class B(x: String) extends A[String]
     | final case class C(x: Int) extends A[Int]

scala> def f[T](a: A[T]): T = a match { case B(_) | C(_) => "plop" }

scala> f(C(1)) + 1
java.lang.ClassCastException: class java.lang.String cannot be cast to class java.lang.Integer (java.lang.String and java.lang.Integer are in module java.base of loader 'bootstrap')
  at scala.runtime.BoxesRunTime.unboxToInt(BoxesRunTime.java:99)

It seems only the constraints from the first pattern are applied, so T = String when typing the case body.

The bug report initially asked if it's possible to support this case:

scala> def f[T](a: A[T]): T = a match {
     |   case B(_) | C(_) => a match { case C(_) => 42 }
     | }
-- [E007] Type Mismatch Error: -------------------------------------------------
2 |  case B(_) | C(_) => a match { case C(_) => 42 }
  |                                             ^^
  |             Found:    (42 : Int)
  |             Required: T
  |
  |             where:    T is a type in method f which is an alias of String
@lrytz lrytz added area:gadt itype:bug itype:soundness Soundness bug (it lets us compile code that crashes at runtime with a ClassCastException) stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 14, 2025
@Gedochao Gedochao removed the stat:needs triage Every issue needs to have an "area" and "itype" label label Mar 14, 2025
@szeiger
Copy link
Contributor

szeiger commented Apr 11, 2025

@lrytz Are you saying that the original example is unsound and should not compile?

Note that the problematic part under the outer match is completely independent from said match. This was not the case in the original problem that I was debugging and I assumed that this was the reason why it failed, but that was not the case. The definition val v2: T = a match { case C(x) => 42 } only refers to the top-level symbols a, x and T which have not been shadowed or re-bound by the match (e.g. case a @ (B(_) | C(_)) =>), and Scala does not have flow-typing, either. Why would v2 be different than the identical definition of v?

sealed trait A[T] { def x: T }
final case class B(x: String) extends A[String]
final case class C(x: Int) extends A[Int]

def f[T](a: A[T]): Unit = {
  def v: T = a match { case C(x) => 42 }
  a match {
    case B(_) | C(_) =>
      val v1: T = v
      val v2: T = a match { case C(x) => 42 }
      ()
  }
}

@Linyxus
Copy link
Contributor

Linyxus commented Apr 11, 2025

scala> sealed trait A[T] { def x: T }
     | final case class B(x: String) extends A[String]
     | final case class C(x: Int) extends A[Int]

scala> def f[T](a: A[T]): T = a match { case B(_) | C(_) => "plop" }

scala> f(C(1)) + 1
java.lang.ClassCastException: class java.lang.String cannot be cast to class java.lang.Integer (java.lang.String and java.lang.Integer are in module java.base of loader 'bootstrap')
  at scala.runtime.BoxesRunTime.unboxToInt(BoxesRunTime.java:99)

is unsound and should be prevented, which is what the associated PR does.

But I agree that this code:

sealed trait A[T] { def x: T }
final case class B(x: String) extends A[String]
final case class C(x: Int) extends A[Int]

def f[T](a: A[T]): Unit = {
  def v: T = a match { case C(x) => 42 }
  a match {
    case B(_) | C(_) =>
      val v1: T = v
      val v2: T = a match { case C(x) => 42 }
      ()
  }
}

is sound and could be supported. Not sure about the status in Scala 2 but Scala 3 compiles it with only pattern matching exclusivity warnings.

@WojciechMazur WojciechMazur added this to the 3.7.1 milestone May 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:gadt itype:bug itype:soundness Soundness bug (it lets us compile code that crashes at runtime with a ClassCastException)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants