Unsound implementation of singleton type matching

[LPTK]

Minimized code

case class Key[T]()(val value: T)

def foo[K[_], A, B](k0: K[A], k1: K[B]): Option[k1.type] = k0 match {
  case k: k1.type => Some(k)
  case _ => None
}

@main def test = {
  val k0 = Key()(42)
  val k1 = Key()("oops")
  foo(k0, k1).foreach(_.value.length)
}

https://scastie.scala-lang.org/2LlP0dwdT7qqFiTTGUSszA

Output

java.lang.ClassCastException: java.lang.Integer cannot be cast to java.lang.String
	at main$package$.test$$anonfun$1(main.scala:12)
	at scala.Option.foreach(Option.scala:437)
	at main$package$.test(main.scala:12)
	at test.main(main.scala:9)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at sbt.Run.invokeMain(Run.scala:115)
	at sbt.Run.execute$1(Run.scala:79)
	at sbt.Run.$anonfun$runWithLoader$4(Run.scala:92)
	at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
	at sbt.util.InterfaceUtil$$anon$1.get(InterfaceUtil.scala:10)
	at sbt.TrapExit$App.run(TrapExit.scala:257)
	at java.lang.Thread.run(Thread.java:748)

Expectation

Type error.

[odersky]

Any suggestion how to solve this?

[smarter]

case k: k1.type => Some(k)

Looks like this is compiled to == k1 when it should be compiled to eq k1 (by contrast k0.isInstanceOf[k1] is correctly compiled to k0 eq k1)

[LPTK]

Yes, the compiler generates the wrong check for this code. I don't have time to look into it ATM, but I could give it a go in about a week.

[bishabosha]

K[_] is not a subtype of AnyRef, so there is no eq method, unless we are meant to use the assumption of erased generics?
https://github.com/lampepfl/dotty/blob/5cdfd31ef55c6e5e93f938d64bde503bae95975f/compiler/src/dotty/tools/dotc/ast/tpd.scala#L948

[smarter]

Interesting, I hadn't seen that code. I think that logic is erroneous: we can only fallback on == if the type of the value is a subtype of AnyVal, otherwise I guess we need to emit an error that this singleton type test is illegal.

[smarter]

(and indeed, the testcase can be rewritten to def foo[K[_] <: AnyRef, ...] to behave appropriately)

[bishabosha]

actually, Scala 2 has the same behaviour (calls BoxesRunTime.equals) and warns about erasure:

example2.scala:5: warning: abstract type pattern K[B] (the underlying of k1.type) is unchecked since it is eliminated by erasure
    case k: k1.type => Some(k)
              ^
1 warning

[smarter]

Ah indeed, this is related to #8430 where we concluded that we could always emit such warnings like Scala 2 does, so I guess that would technically solve this issue too (but I'm not supper happy with the fact that adding/removing a <: AnyRef bound can silently changes semantics like this).

[bishabosha]

this seems to work fine:

if (tp.widen.derivesFrom(defn.AnyValClass))
  singleton(tp).equal(tree)
else
  tree.ensureConforms(defn.ObjectType).select(defn.Object_eq).appliedTo(singleton(tp).ensureConforms(defn.ObjectType))

and the generated code seems fine:

public <K, A, B> Option<Object> foo(Object k0, Object k1) {
    None$ none$;
    Object object = k0;
    if (object == k1) {
        Object k = object;
        none$ = Some$.MODULE$.apply(k);
    } else {
        none$ = None$.MODULE$;
    }
    return none$;
}

[smarter]

We can't do that, because if you have a primitive (or value class) upcasted to Any, it should not be compared using eq, because the same primitive might be boxed to two different objects.

[LPTK]

Isn't there a way to test, at runtime, whether two Any values have the same identity? For references it would use eq, and for boxed AnyVal values it would use ==.

[smarter]

For primitives, you could hardcode a list, but for non-primitive value classes I don't see a way. As always the answer is: valhalla will solve everything :).

[LPTK]

Note that AnyVal classes which happen to be equal (by equals) should not match with .type patterns, as that would be unsound (they could have differing type arguments/type members). So the current behavior is probably wrong:

scala> class Test(val x: Int) extends AnyVal
// defined class Test

scala> val t = new Test(1)
val t: Test = 1

scala> val u = new Test(1)
val u: Test = 1

scala> t match { case x: u.type => true }
val res0: Boolean = true

Now, AnyVal instances do not have a stable identity, since they create new boxes upon being up-casted to Any. So I think .type tests should simply never return true for AnyVal classes, should they be boxed or not.