Could the compiler help me catch this?

[Bjorn Regnell]

Can someone explain why the type of val unsafe below is allowed to be untrue... Could (the || a) compiler have helped me before run-time disaster broke loose?
(I know erasure is lurking here, but it would have been cool if the type inference engine had protested or inferred Vector[Box[Any]] or something)
/Bjorn

Welcome to Scala version 2.10.4 (Java HotSpot(TM) 64-Bit Server VM, Java 1.7.0_51).
Type in expressions to have them evaluated.
Type :help for more information.

scala> :paste
// Entering paste mode (ctrl-D to finish)

case class Box[T](x: T)
val boxes: Vector[Any] = Vector(Box(3), Box("x"), Box(3.14))
def strange[T](b: Box[T]): Vector[Box[T]] = boxes collect { case b: Box[T] => b }

// Exiting paste mode, now interpreting.

defined class Box
boxes: Vector[Any] = Vector(Box(3), Box(x), Box(3.14))
strange: [T](b: Box[T])Vector[Box[T]]

scala> val unsafe = strange(Box(42))  //unsafe gets untrue type
unsafe: Vector[Box[Int]] = Vector(Box(3), Box(x), Box(3.14))

scala> unsafe(2).x  //run-time kaboom...
java.lang.ClassCastException: java.lang.Double cannot be cast to java.lang.Integer
        at scala.runtime.BoxesRunTime.unboxToInt(BoxesRunTime.java:106)
        at .<init>(<console>:13)

[Oliver Ruebenacker]

     Hello,

  Related:

scala> val l = List(1, 1.1, "a")
l: List[Any] = List(1, 1.1, a)

scala> def f[T](x:T, l:List[Any]) = l collect { case y:T => y }
<console>:8: warning: abstract type pattern T is unchecked since it is eliminated by erasure
       def f[T](x:T, l:List[Any]) = l collect { case y:T => y }
                                                       ^
f: [T](x: T, l: List[Any])List[T]

scala> f(1, l)
res2: List[Int] = List(1, 1.1, a)

     Best,

     Oliver

--
You received this message because you are subscribed to the Google Groups "scala-user" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-user+...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Oliver Ruebenacker

Founder at Relomics Consulting

Be always grateful, but never satisfied.

[Oliver Ruebenacker]

     Hello,

  If I specify a return type, I can get a class cast exception, too.

  I guess the lesson is that using type parameters in partial functions defeats type safety.

scala> val l = List(1, 1.1, "a")
l: List[Any] = List(1, 1.1, a)

scala> def f[T](x:T, l:List[Any]) : List[T] = l collect { case y:T => y }

<console>:8: warning: abstract type pattern T is unchecked since it is eliminated by erasure

       def f[T](x:T, l:List[Any]) : List[T] = l collect { case y:T => y }

                                                                 ^
f: [T](x: T, l: List[Any])List[T]

scala> val l2 = f(1, l)
l2: List[Int] = List(1, 1.1, a)

scala> val l3 = l2.map(2*_)

java.lang.ClassCastException: java.lang.Double cannot be cast to java.lang.Integer
    at scala.runtime.BoxesRunTime.unboxToInt(BoxesRunTime.java:106)

    at $anonfun$1.apply(<console>:10)
    at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
    at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
    at scala.collection.immutable.List.foreach(List.scala:318)
    at scala.collection.TraversableLike$class.map(TraversableLike.scala:244)
    at scala.collection.AbstractTraversable.map(Traversable.scala:105)
    at .<init>(<console>:10)
    at .<clinit>(<console>)
    at .<init>(<console>:7)
    at .<clinit>(<console>)
    at $print(<console>)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at scala.tools.nsc.interpreter.IMain$ReadEvalPrint.call(IMain.scala:734)
    at scala.tools.nsc.interpreter.IMain$Request.loadAndRun(IMain.scala:983)
    at scala.tools.nsc.interpreter.IMain.loadAndRunReq$1(IMain.scala:573)
    at scala.tools.nsc.interpreter.IMain.interpret(IMain.scala:604)
    at scala.tools.nsc.interpreter.IMain.interpret(IMain.scala:568)
    at scala.tools.nsc.interpreter.ILoop.reallyInterpret$1(ILoop.scala:756)
    at scala.tools.nsc.interpreter.ILoop.interpretStartingWith(ILoop.scala:801)
    at scala.tools.nsc.interpreter.ILoop.command(ILoop.scala:713)
    at scala.tools.nsc.interpreter.ILoop.processLine$1(ILoop.scala:577)
    at scala.tools.nsc.interpreter.ILoop.innerLoop$1(ILoop.scala:584)
    at scala.tools.nsc.interpreter.ILoop.loop(ILoop.scala:587)
    at scala.tools.nsc.interpreter.ILoop$$anonfun$process$1.apply$mcZ$sp(ILoop.scala:878)
    at scala.tools.nsc.interpreter.ILoop$$anonfun$process$1.apply(ILoop.scala:833)
    at scala.tools.nsc.interpreter.ILoop$$anonfun$process$1.apply(ILoop.scala:833)
    at scala.tools.nsc.util.ScalaClassLoader$.savingContextLoader(ScalaClassLoader.scala:135)
    at scala.tools.nsc.interpreter.ILoop.process(ILoop.scala:833)
    at scala.tools.nsc.MainGenericRunner.runTarget$1(MainGenericRunner.scala:83)
    at scala.tools.nsc.MainGenericRunner.process(MainGenericRunner.scala:96)
    at scala.tools.nsc.MainGenericRunner$.main(MainGenericRunner.scala:105)
    at scala.tools.nsc.MainGenericRunner.main(MainGenericRunner.scala)


scala>

     Best,

     Oliver

[Björn Regnell]

But you at least got a warning... I was getting the silent treatment by the compiler ;)

/Bjorn

[Jason Zaugg]

That looks like a (serious) bug to me. I've lodged it as SI-8597.

-jason

[Björn Regnell]

Aha! Good to see it marked critical. But the bug text complains only about the absent warning. What about the false type inferred in my example? Shouldn't the type be Vector[Box[Any]] or something better than Vector[Box[Int]] which is clearly misleading...?

/Bjorn

 

 

From: scala...@googlegroups.com [mailto:scala...@googlegroups.com] On Behalf Of Jason Zaugg
Sent: den 16 maj 2014 19:05
To: Björn Regnell
Cc: scala-user
Subject: Re: [scala-user] Could the compiler help me catch this?

 

That looks like a (serious) bug to me. I've lodged it as SI-8597.

 

-jason

[Haoyi Li]

> scala> f(1, l)
> res2: List[Int] = List(1, 1.1, a)

That's terrible, like a billion proofs cried out and were suddenly silenced 

--

[Oliver Ruebenacker]

The type is correct. The problem is that your partial function matched objects it should not match, because it could not tell the difference due to erasure.

--

[Björn Regnell]

How can this type be correct? (Or do I have too high hopes of the regularity of reality?)

Vector[Box[Int]] = Vector(Box(3), Box(x), Box(3.14))

 

[Jason Zaugg]

On Fri, May 16, 2014 at 8:57 PM, Björn Regnell <bjorn....@cs.lth.se> wrote:

Aha! Good to see it marked critical. But the bug text complains only about the absent warning. What about the false type inferred in my example? Shouldn't the type be Vector[Box[Any]] or something better than Vector[Box[Int]] which is clearly misleading...?

/Bjorn

At the call site, inference only considers the declared signature:

def strange[T](b: Box[T]): Vector[Box[T]] 

So if we call this with an argument of type `Box[Int]`, the type argument is inferred as `Int` and the result type as `Vector[Box[Int]]`.

The lie is in the body of strange  

 

-jason

[Björn Regnell]

Yes. But what is the compiler supposed to do when the bug is fixed? Just warn me or refuse?

If it just warns me and the code still compiles, then the inferred type would be the wrong one still?

/Bjorn

 

From: Jason Zaugg [mailto:jza...@gmail.com]

Sent: den 16 maj 2014 21:34
To: Björn Regnell
Cc: scala-user

[Haoyi Li]

I'd prefer that it refuse. If I wanted an unchecked cast, I'd put an unchecked cast; a type-case-that-doesn't-do-anything is a lie.

--

[Jason Zaugg]

It will be an unchecked warning at the place you pattern match.

We're working on a way for you to configure escalations or demotions of warnings, which will let you have to discipline of `-Xfatal-warnings` with the requisite escape hatch for spots when you know better or when the compiler warning is itself buggy.

-jason

[Haoyi Li]

If we want an escape hatch, couldn't we just pattern match against 

case x =>

case x: Seq[_] =>

instead of 

case x: T =>

case x: Seq[T] =>

and then just cast it ourselves after?

[Jason Zaugg]

On Fri, May 16, 2014 at 9:47 PM, Haoyi Li <haoy...@gmail.com> wrote:

If we want an escape hatch, couldn't we just pattern match against 

case x =>

case x: Seq[_] =>

instead of 

case x: T =>

case x: Seq[T] =>

and then just cast it ourselves after?

Actually, you can escape from an unchecked warning today as follows:

scala> (Nil: List[Any]) match { case is: List[Int] => is }

<console>:8: warning: non-variable type argument Int in type pattern List[Int] is unchecked since it is eliminated by erasure

              (Nil: List[Any]) match { case is: List[Int] => is }

                                                ^

res0: List[Int] = List()

scala> (Nil: List[Any]) match { case is: List[Int @unchecked] => is }

res1: List[Int @unchecked] = List()

What is missing is a way to escalate unchecked warnings to errors. But that will be available soon.

It wasn't so long ago that these warning were more or less hidden by default:

% scala29

Welcome to Scala version 2.9.2 (Java HotSpot(TM) 64-Bit Server VM, Java 1.6.0_65).

Type in expressions to have them evaluated.

Type :help for more information.

scala> (Nil: List[Any]) match { case is: List[Int] => is }

warning: there were unchecked warnings; re-run with -unchecked for details

res0: List[Int] = List()

[Haoyi Li]

I was more thinking of making errors the *default*, and then using manual casting as the "escape":

scala> (Nil: List[Any]) match { case is: List[_] => is.asInstanceOf[List[Int]] }

I mean, we already have a way of making unchecked type assertions, it's called casting =P Unless I'm missing something, @unchecked and similar don't actually provide any new capability, just a slightly-different syntax to do the same thing. My theory is that these sort of escape-hatches would be used rarely enough that a bit of verbosity isn't too bad.

[Jason Zaugg]

On Fri, May 16, 2014 at 10:26 PM, Haoyi Li <haoy...@gmail.com> wrote:

I was more thinking of making errors the *default*, and then using manual casting as the "escape":

scala> (Nil: List[Any]) match { case is: List[_] => is.asInstanceOf[List[Int]] }

I mean, we already have a way of making unchecked type assertions, it's called casting =P Unless I'm missing something, @unchecked and similar don't actually provide any new capability, just a slightly-different syntax to do the same thing. My theory is that these sort of escape-hatches would be used rarely enough that a bit of verbosity isn't too bad.

You'd have to write the cast twice if you refer the the bound pattern in a guard and in a case body. But that's not such a strong argument. But IIRC, adding the shorter syntax for unchecked type patterns was a prerequisite for turning the unchecked warnings on by default (the other was fixing some false positives), so we can be thankful for that.

Incidentally, sometimes we can allow type arguments in a type pattern:

class C; class D extends C

def okay = (List(new D) : Seq[D]) match { case _: List[C] => case _ => } 

We still need a runtime type test to check if the `Seq` is a `List`, but once we know that we can be statically assured that will be a subtype of `List[C]`. The compiler properly takes this into account since Scala 2.10.

-jason

[Haoyi Li]

> You'd have to write the cast twice if you refer the the bound pattern in a guard and in a case body

Somewhat off topic, but this seems like something parametrized extractors would have fixed, since you could use an identity extractor (pass in an expression as the parameter, get the value of the expression as a name) to bind the casted value to a name that could be used later in the pattern/guard/body. I remember seeing those on the 2.11 roadmap. Any idea what happened to them?

[Jason Zaugg]

On Fri, May 16, 2014 at 11:02 PM, Haoyi Li <haoy...@gmail.com> wrote:

> You'd have to write the cast twice if you refer the the bound pattern in a guard and in a case body

Somewhat off topic, but this seems like something parametrized extractors would have fixed, since you could use an identity extractor (pass in an expression as the parameter, get the value of the expression as a name) to bind the casted value to a name that could be used later in the pattern/guard/body. I remember seeing those on the 2.11 roadmap. Any idea what happened to them?

We once discussed opening the door to arbitrary expressions in patterns, rather than just stable references to extractors.

The counter argument to this is that the pattern matcher doesn't guarantee the same left-to-right evaluation for patterns as one is used to for expressions.

As a bit of a contrived example, see if you can figure out what the following program prints (any why!). What if you match on `"abc": Any` instead?

object ShoutyString {

  println("SS.init")

  def unapply(s: String) = s == s.toUpperCase

}

object Quiet {

  println("Q.init")

  def unapply(s: Any) =

    s match {

      case s: String => s == s.toLowerCase

      case _ => false

    }

}

object Test extends App {

  (23: Any) match {

   case x: String if false =>

   case ShoutyString() | Quiet() =>

   case _ =>

  }

}

These minor surprises come from the way that pattern matcher can short circuit parts of patterns based on knowledge it gleans from evaluating the previous ones. It eliminates common sub expressions and redundant type tests.

Now for stable references to extractors the surprise factor is pretty low, as only side effects are pretty limited. But if we let you write:

   case {quietOrShouty(true)}() | {quietOrShouty(false)() =>

It starts to look a lot more like code the people expect to be evaluated left to right, and evaluated one only. So that's what we need to be mindful of if we loosen this restriction. It's not currently on our 2.12 roadmap.

-jason

[Jason Zaugg]

On Sat, May 17, 2014 at 12:00 AM, Jason Zaugg <jza...@gmail.com> wrote:

On Fri, May 16, 2014 at 11:02 PM, Haoyi Li <haoy...@gmail.com> wrote:

> You'd have to write the cast twice if you refer the the bound pattern in a guard and in a case body

Somewhat off topic, but this seems like something parametrized extractors would have fixed, since you could use an identity extractor (pass in an expression as the parameter, get the value of the expression as a name) to bind the casted value to a name that could be used later in the pattern/guard/body. I remember seeing those on the 2.11 roadmap. Any idea what happened to them?

Oh, and back on topic, I think I've found the spot to fix this bug. 

Will run it by Adriaan next week. Thanks for reporting!

-jason

[Björn Regnell]

> Thanks for reporting!

And thanks for taking time to explain, Jason!

After a second thought, your devised scheme of escalation of warnings seems good. I guess if the compiler would refuse there would be situations that would be too restrictive. A warning is fine by me.

/Bjorn