Skip to content

Unexpected behavior for pattern matching on generic case class #5077

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
grindvoll opened this issue Sep 4, 2018 · 4 comments · Fixed by #10672
Closed

Unexpected behavior for pattern matching on generic case class #5077

grindvoll opened this issue Sep 4, 2018 · 4 comments · Fixed by #10672

Comments

@grindvoll
Copy link

The following example defines a simple case class C with two members and one generic type. Pattern matching on this class gives some surprising results in various cases, and the results differ significantly from 2.12/2.13 behavior. (In fact, all the three cases below are treated differently in Dotty vs. 2.12/2.13):

trait Is[A]
case object IsInt extends Is[Int]
case object IsString extends Is[String]
case class C[A](is: Is[A], value: A)

val c_string: C[String] = C(IsString, "name")
val c_any: C[_] = c_string
val any: Any = c_string

// Case 1: Should give compile error, since IsInt is not compatible with C[String]
c_string match {
  case C(IsInt, _) => println(s"An Int") // Can't possibly happen!
  case C(IsString, s) => println(s"A String with length ${s.length}")
  case _ => println("No match")
}

// Case 2: Should match the second case and print the length of the string
c_any match {
  case C(IsInt, i) if i < 10 => println(s"An Int less than 10")
  case C(IsString, s) => println(s"A String with length ${s.length}")
  case _ => println("No match")
}

// Case 3: Same as above; should match the second case and print the length of the string
any match {
  case C(IsInt, i) if i < 10 => println(s"An Int less than 10")
  case C(IsString, s) => println(s"A String with length ${s.length}")
  case _ => println("No match")
}

In Dotty 0.9.0-RC, only Case 3 is handled as expected (which is the only case that isn't handled correctly in 2.12/2.13). The actual result is that Case 2 fails to compile:

[error] -- [E008] Member Not Found Error: C:\dev\dottytest\src\main\scala\Main.scala:23:28
[error] 23 |      case C(IsInt, i) if i < 10 => println(s"An Int less than 10")
[error]    |                          ^^^
[error]    |              value `<` is not a member of C[_]#A - did you mean `i.==`?
[error] -- [E008] Member Not Found Error: C:\dev\dottytest\src\main\scala\Main.scala:24:64
[error] 24 |      case C(IsString, s) => println(s"A String with length ${s.length}")
[error]    |                                                              ^^^^^^^^
[error]    |                                value `length` is not a member of C[_]#A
[error] two errors found

I would assume the behavior to be:

Case 1

This case should be rejected by the compiler since the IsInt member is not compatible with C[String]. But Dotty accepts this, while 2.12 correctly rejects the pattern match due to incompatible types.

Case 2

The type being matched is a C[_]. 2.12 accepts this pattern match and works as expected, while Dotty does not figure out the type of the corresponding value member. (I am not sure if this is expected in Dotty due to removal of existential types, but it is nevertheless surprising).

Case 3

This compiles and Dotty is able to infer the type of the value based on the case object given as the first parameter. However, in 2.12 this fails (see: Scala 2.12 bug 11132)

@liufengyun
Copy link
Contributor

liufengyun commented Jun 2, 2020

We encounter an erasure problem for the code below:

c_any match {
  case C(IsInt, i) if i < 10 => println(s"An Int less than 10")
  case C(IsString, s) => println(s"A String with length ${s.length}")
  case _ => println("No match")
}

It desugars to:

                case val x15: C[A$1] =
                  C.unapply[A$1 @ A$1](x9.$asInstanceOf$[C[A$1]])
                case val x16: Is[A$1] = x15._1
                case val x17: A$1 = x15._2
                if IsInt.==(x16) then
                  {
                    case val i: A$1 = x17
                    if i.<(10) then
                      return[matchResult8]
                        {
                          println("An Int less than 10")
                        }
                     else ()
                  }
                 else ()

In the above, case val x17: A$1 = x15._2 erases to case val x17: Int = x15._2:

                case val x15: C = C.unapply(x9:C)
                case val x16: Is = x15._1()
                case val x17: Int = scala.Int.unbox(x15._2())
                if IsInt.==(x16) then
                  {
                    case val i: Int = x17
                    if i.<(10) then
                      {
                        return[matchResult8] println("An Int less than 10")
                      }
                     else ()
                  }
                 else ()

which causes runtime exception if IsInt.==(x16) is false:

 cast to java.lang.Integer
	at scala.runtime.BoxesRunTime.unboxToInt(BoxesRunTime.java:99)
	at i5077$package$.Test(i5077.scala:21)
	at Test.main(i5077.scala:6)

@smarter
Copy link
Member

smarter commented Jun 2, 2020

Sounds like we need to move the generation of case val x17: A$1 = x15._2 inside the if IsInt.==(x16) then since that check is what allows us to know the precise type of x15._2 ?

@liufengyun
Copy link
Contributor

liufengyun commented Jun 2, 2020

@smarter Generally we cannot do that, as x17 might be used in the guard.

Edited: I was wrong, the test is not the guard.

liufengyun added a commit to dotty-staging/dotty that referenced this issue Dec 6, 2020
Given the following definition:

    trait Is[A]
    case object IsInt extends Is[Int]
    case object IsString extends Is[String]
    case class C[A](is: Is[A], value: A)

and the pattern match:

    (x: C[_]) match
    case C(IsInt, i) => ...

The typer (enhanced with GADT) will infer `C[A$1]` as the type of the
pattern, where `A$1 =:= Int`.

The patternMatcher generates the following code:

    case val x15: C[A$1] =
      C.unapply[A$1 @ A$1](x9.$asInstanceOf$[C[A$1]])
    case val x16: Is[A$1] = x15._1
    case val x17: A$1 = x15._2        // erase to `Int`
    if IsInt.==(x16) then
      {
        case val i: A$1 = x17
	...
      }

Note that `x17` will have the erased type `Int`. This is incorrect: it
may only assume the type `Int` if the test is true inside the block.
If the test is false, we will get an type cast exception at runtime.

To fix the problem, we replace pattern-bound types by `Any` for
selector results:

    case val x15: C[A$1] =
      C.unapply[A$1 @ A$1](x9.$asInstanceOf$[C[A$1]])
    case val x16: Is[A$1] = x15._1
    case val x17: Any = x15._2
    if IsInt.==(x16) then
      {
        case val i: A$1 = x17.$asInstanceOf$[A$1]
        ...
      }

The patternMatcher will then use a type cast to pass the selector
value for nested unapplys or assign it to bound variables.
@liufengyun
Copy link
Contributor

For the first pattern match, it is actually correct for the compiler to not issue an error:

// Case 1: Should give compile error, since IsInt is not compatible with C[String]
c_string match {
  case C(IsInt, _) => println(s"An Int") // Can't possibly happen!
  case C(IsString, s) => println(s"A String with length ${s.length}")
  case _ => println("No match")
}

The reason is that the equals method of IsInt might be overridden to match IsString. Related #9740.

liufengyun added a commit to dotty-staging/dotty that referenced this issue Dec 9, 2020
Given the following definition:

    trait Is[A]
    case object IsInt extends Is[Int]
    case object IsString extends Is[String]
    case class C[A](is: Is[A], value: A)

and the pattern match:

    (x: C[_]) match
    case C(IsInt, i) => ...

The typer (enhanced with GADT) will infer `C[A$1]` as the type of the
pattern, where `A$1 =:= Int`.

The patternMatcher generates the following code:

    case val x15: C[A$1] =
      C.unapply[A$1 @ A$1](x9.$asInstanceOf$[C[A$1]])
    case val x16: Is[A$1] = x15._1
    case val x17: A$1 = x15._2        // erase to `Int`
    if IsInt.==(x16) then
      {
        case val i: A$1 = x17
	...
      }

Note that `x17` will have the erased type `Int`. This is incorrect: it
may only assume the type `Int` if the test is true inside the block.
If the test is false, we will get an type cast exception at runtime.

To fix the problem, we replace pattern-bound types by `Any` for
selector results:

    case val x15: C[A$1] =
      C.unapply[A$1 @ A$1](x9.$asInstanceOf$[C[A$1]])
    case val x16: Is[A$1] = x15._1
    case val x17: Any = x15._2
    if IsInt.==(x16) then
      {
        case val i: A$1 = x17.$asInstanceOf$[A$1]
        ...
      }

The patternMatcher will then use a type cast to pass the selector
value for nested unapplys or assign it to bound variables.
liufengyun added a commit to dotty-staging/dotty that referenced this issue Dec 9, 2020
Given the following definition:

    trait Is[A]
    case object IsInt extends Is[Int]
    case object IsString extends Is[String]
    case class C[A](is: Is[A], value: A)

and the pattern match:

    (x: C[_]) match
    case C(IsInt, i) => ...

The typer (enhanced with GADT) will infer `C[A$1]` as the type of the
pattern, where `A$1 =:= Int`.

The patternMatcher generates the following code:

    case val x15: C[A$1] =
      C.unapply[A$1 @ A$1](x9.$asInstanceOf$[C[A$1]])
    case val x16: Is[A$1] = x15._1
    case val x17: A$1 = x15._2        // erase to `Int`
    if IsInt.==(x16) then
      {
        case val i: A$1 = x17
	...
      }

Note that `x17` will have the erased type `Int`. This is incorrect: it
may only assume the type `Int` if the test is true inside the block.
If the test is false, we will get an type cast exception at runtime.

To fix the problem, we replace pattern-bound types by `Any` for
selector results:

    case val x15: C[A$1] =
      C.unapply[A$1 @ A$1](x9.$asInstanceOf$[C[A$1]])
    case val x16: Is[A$1] = x15._1
    case val x17: Any = x15._2
    if IsInt.==(x16) then
      {
        case val i: A$1 = x17.$asInstanceOf$[A$1]
        ...
      }

The patternMatcher will then use a type cast to pass the selector
value for nested unapplys or assign it to bound variables.
liufengyun added a commit to dotty-staging/dotty that referenced this issue Dec 10, 2020
Given the following definition:

    trait Is[A]
    case object IsInt extends Is[Int]
    case object IsString extends Is[String]
    case class C[A](is: Is[A], value: A)

and the pattern match:

    (x: C[_]) match
    case C(IsInt, i) => ...

The typer (enhanced with GADT) will infer `C[A$1]` as the type of the
pattern, where `A$1 =:= Int`.

The patternMatcher generates the following code:

    case val x15: C[A$1] =
      C.unapply[A$1 @ A$1](x9.$asInstanceOf$[C[A$1]])
    case val x16: Is[A$1] = x15._1
    case val x17: A$1 = x15._2        // erase to `Int`
    if IsInt.==(x16) then
      {
        case val i: A$1 = x17
	...
      }

Note that `x17` will have the erased type `Int`. This is incorrect: it
may only assume the type `Int` if the test is true inside the block.
If the test is false, we will get an type cast exception at runtime.

To fix the problem, we replace pattern-bound types by `Any` for
selector results:

    case val x15: C[A$1] =
      C.unapply[A$1 @ A$1](x9.$asInstanceOf$[C[A$1]])
    case val x16: Is[A$1] = x15._1
    case val x17: Any = x15._2
    if IsInt.==(x16) then
      {
        case val i: A$1 = x17.$asInstanceOf$[A$1]
        ...
      }

The patternMatcher will then use a type cast to pass the selector
value for nested unapplys or assign it to bound variables.
liufengyun added a commit that referenced this issue Dec 10, 2020
Fix #5077: avoid pattern-bound type for selectors
@Kordyjan Kordyjan added this to the 3.0.0 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants