Scala Programming Language
  1. Scala Programming Language
  2. SI-5407

Private val in case class leads to either to weird compilation error or ClassCastException

    Details

      Description

      PrivateVal.scala
      case class Foo(private val x: Int, y: Option[Int], z: Boolean)
      object PrivateVal extends App {
        def foo(x: Foo) = x match {
          case Foo(x, Some(y), z) => y
          case Foo(x, y, z) => 0
        }
        val x = Foo(1, Some(2), false)
        println(foo(x))
      }
      

      Compiling using 2.10.0-M1 gives us:

      PrivateVal.scala:3: error: isInstanceOf cannot test if value types are references.
        def foo(x: Foo) = x match {
                            ^
      one error found
      

      if we change method foo to

      def foo(x: Foo) = x match {
        case Foo(x, Some(y), z) => y
        case Foo(x, None, z) => 0
      }
      

      the code compiles but throws CCE at runtime:

      /scala PrivateVal
      java.lang.ClassCastException: java.lang.Boolean cannot be cast to scala.Option
      	at PrivateVal$.foo(PrivateVal.scala:4)
      	at PrivateVal$delayedInit$body.apply(PrivateVal.scala:9)
      

      Ouch! If you compile it with -Xprint:cleanup you'll notice:

      def foo(x: Foo): Int = {
            <synthetic> val temp2: Foo = x;
            if (temp2.ne(null))
              {
                <synthetic> val temp4: Boolean = temp2.z();
                val y: Option = scala.Boolean.box(temp4).$asInstanceOf[Option]();
                if (PrivateVal.this.gd1$1(y))
      ...
      

      Which means pattern matching is completely confused about the arity (and thus types) of our case class.

      Both definitions of foo this worked in 2.9.x so it's a regression. However, the whole idea of having private vals in case classes feels weird. We declare a member to be private but then we can access it through a pattern match (in 2.9.x). I think we should simply disallow private vals in case class's primary constructor. Proposed way to move forward:
      1. bring back questionable semantics of 2.9.x back to master
      2. Deprecate private vals in primary constructor of a case class
      3. Disallow it in the next major release of Scala

      PS. Yvirtpatmat fails to handle private vals properly as well.
      PS2. The test-case has been extracted with blood and tears from specs2 by me and Nada Amin.

        Activity

        Hide
        Paul Phillips added a comment -

        > I think we should simply disallow private vals in case class's primary constructor.

        Been there, did that, purchased the t-shirts, discovered this,

        final case class ::[B](private var hd: B, private[scala] var tl: List[B]) extends List[B]
        

        returned to drawing board.

        Unless you mean "disallow private vals but not private vars", but it seems unlikely you mean that since it would be inexplicable and also wouldn't remove any problem we have.

        Show
        Paul Phillips added a comment - > I think we should simply disallow private vals in case class's primary constructor. Been there, did that, purchased the t-shirts, discovered this, final case class ::[B](private var hd: B, private[scala] var tl: List[B]) extends List[B] returned to drawing board. Unless you mean "disallow private vals but not private vars", but it seems unlikely you mean that since it would be inexplicable and also wouldn't remove any problem we have.
        Hide
        Hubert Plociniczak added a comment -

        re 2) I remember we had similar discussion with private[this], which we disallowed. I don't remember the ticket but there might be some discussion related to private vals as well.

        Show
        Hubert Plociniczak added a comment - re 2) I remember we had similar discussion with private [this] , which we disallowed. I don't remember the ticket but there might be some discussion related to private vals as well.
        Hide
        Paul Phillips added a comment -

        Aha, I found it. Please see the comments in https://issues.scala-lang.org/browse/SI-3714 .

        Show
        Paul Phillips added a comment - Aha, I found it. Please see the comments in https://issues.scala-lang.org/browse/SI-3714 .
        Hide
        Jason Zaugg added a comment -

        This only affects patmatclassic.

        https://github.com/scala/scala/pull/535

        Show
        Jason Zaugg added a comment - This only affects patmatclassic. https://github.com/scala/scala/pull/535
        Hide
        Jason Zaugg added a comment -

        Closing, as this works with the virtpatmat.

        Show
        Jason Zaugg added a comment - Closing, as this works with the virtpatmat.

          People

          • Assignee:
            Jason Zaugg
            Reporter:
            Grzegorz Kossakowski
          • Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development