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

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

Closed
scabug opened this Issue Jan 25, 2012 · 6 comments

Comments

Projects
None yet
2 participants
@scabug
Copy link

scabug commented Jan 25, 2012

// 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.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jan 25, 2012

Imported From: https://issues.scala-lang.org/browse/SI-5407?orig=1
Reporter: @gkossakowski
Affected Versions: 2.10.0
Other Milestones: 2.10.0

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jan 25, 2012

@paulp said:

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.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jan 25, 2012

@hubertp said:
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.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jan 25, 2012

@paulp said:
Aha, I found it. Please see the comments in #3714 .

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 12, 2012

@retronym said:
This only affects patmatclassic.

scala/scala#535

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 20, 2012

@retronym said:
Closing, as this works with the virtpatmat.

@scabug scabug closed this May 20, 2012

@scabug scabug added this to the 2.10.0-M3 milestone Apr 7, 2017

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