No compile time avoidance of mismatched outers

[som-snytt]

Compiler version

3 dawg

Minimized code

➜  snips cat outerly.scala

import PartialFunction.cond

class C {
  class D(val n: Int) { override def equals(other: Any) = cond(other) { case that: D => that.n == n } }
  val d: D = new D(42)
}

object Test extends App {
  val c1, c2 = new C
  assert(c1.d == c2.d)
  c1.d match {
    case c2.d =>
  }
}

Output

➜  snips scala outerly.scala
Exception in thread "main" java.lang.AssertionError: assertion failed
        at scala.runtime.Scala3RunTime$.assertFailed(Scala3RunTime.scala:11)
        at Test$.<clinit>(outerly.scala:11)
        at Test.main(outerly.scala)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:567)
        at dotty.tools.scripting.ScriptingDriver.compileAndRun(ScriptingDriver.scala:39)
        at dotty.tools.scripting.Main$.main(Main.scala:39)
        at dotty.tools.scripting.Main.main(Main.scala)
➜  snips sdk use scala 2.13.6

Using scala version 2.13.6 in this shell.
➜  snips scala outerly.scala
outerly.scala:13: error: type mismatch;
 found   : Main.c2.D
 required: Main.c1.D
    case c2.d =>
            ^

Expectation

I kind of prefer the compile-time error to runtime.

[smarter]

Scala 2 is too conservative and prevents code that would work fine at runtime from compiling:

class C {
  class D
  val d: D = new D
}

object Test extends App {
  def foo(c1: C, c2: C): Unit = {
    c1.d match {
      case c2.d => // OK in Scala 3, type mismatch in Scala 2
    }
  }

  val c = new C
  foo(c, c)
}

On the other hand, the pattern match:

  c1.d match {
    case c2.d =>
  }

should emit an exhaustivity warning since there's no guarantee that the case matches the selector. @liufengyun how does the exhaustivity checker deals with prefixes currently?

[liufengyun]

should emit an exhaustivity warning since there's no guarantee that the case matches the selector. @liufengyun how does the exhaustivity checker deals with prefixes currently?

The check reason at the level of types, so prefixes are handled as part of the type system. In child instantiation, we do prefix inference for child classes (in TypeOps.instantiateToSubtype).

If we add sealed to class D, the compiler reports an expected warning:

-- [E029] Pattern Match Exhaustivity Warning: examples/hello.scala:8:7 ---------
8 |    c1.d match {
  |    ^^^^
  |    match may not be exhaustive.
  |
  |    It would fail on pattern case: _: c1.D

[smarter]

Ah thank you, I forgot about the need for sealed, we also get a warning with -Ycheck-all-patmat so I think we can close this as working as intended (although it's yet another reason why we should consider enabling-Ycheck-all-patmat by default).

[som-snytt]

Surprised TIL D is not effectively sealed. Very surprised there is no warning at all. Willing to entertain which warning is appropriate.

[som-snytt]

This is totally wrong. You have to warn at compile time, basta. There are lots of type checks that wouldn't happen to fail at runtime, but that doesn't mean you don't issue the type check at compile time. I shouldn't match D from arbitrary C, right?

import PartialFunction.cond

class C {
  class D(val n: Int) { override def equals(other: Any) = cond(other) { case that: D => that.n == n } }
  val d: D = new D(42)
}

object Test extends App {
  val c1, c2 = new C
  val x = new c1.D(42) { override def toString = "dope" }
  x match {
    case c1.d =>
  }
  x match {
    case c2.d =>  // fail here
  }
  c1.d match {
    case c2.d =>
  }
  assert(c1.d == c2.d)
}

where this is why REPL needs :javap

➜  snips javap -private -verbose '/tmp/C$$anon$1.class'
  public final java.lang.Object applyOrElse(java.lang.Object, scala.Function1);
    descriptor: (Ljava/lang/Object;Lscala/Function1;)Ljava/lang/Object;
    flags: (0x0011) ACC_PUBLIC, ACC_FINAL
    Code:
      stack=2, locals=5, args_size=3
         0: aload_1
         1: astore_3
         2: aload_3
         3: instanceof    #10                 // class C$D
         6: ifeq          58
         9: aload_3
        10: checkcast     #10                 // class C$D
        13: invokevirtual #36                 // Method C$D.C$D$$$outer:()LC;
        16: aload_0
        17: getfield      #27                 // Field $outer:LC$D;
        20: invokevirtual #36                 // Method C$D.C$D$$$outer:()LC;
        23: if_acmpne     58      // <- outer check
        26: aload_3
        27: checkcast     #10                 // class C$D
        30: astore        4
        32: aload         4
        34: invokevirtual #47                 // Method C$D.n:()I
        37: aload_0
        38: getfield      #27                 // Field $outer:LC$D;
        41: invokevirtual #47                 // Method C$D.n:()I
        44: if_icmpne     51
        47: iconst_1
        48: goto          52
        51: iconst_0
        52: invokestatic  #53                 // Method scala/runtime/BoxesRunTime.boxToBoolean:(Z)Ljava/lang/Boolean;
        55: goto          68
        58: aload_2
        59: aload_1
        60: invokeinterface #59,  2           // InterfaceMethod scala/Function1.apply:(Ljava/lang/Object;)Ljava/lang/Object;
        65: goto          68
        68: areturn

[smarter]

I mean, the whole point of pattern matching is that there might be information you can find at runtime that is unknown at compile-time, so the compiler should only prevent you from writing a pattern match if it's certain that it will in fact never match (implemented by https://github.com/lampepfl/dotty/blob/515cb9f01c609f59e2dd305107596066b0e1f580/compiler/src/dotty/tools/dotc/typer/Typer.scala#L3868-L3872). I do agree that we should have some kind of warning here by default, probably by enabling exhaustivity checking for unsealed selectors by default, but that requires some more discussion: scala/scala#9299 (comment)

[som-snytt]

OK, I see I confused matching on any, which arbitrarily narrows to a subset of values, and an ADT, which narrows to those subtypes. I'm not normally confused on that score, but I was just looking at how uncheckable type args work, namely, when can you accept Seq(42) match { case _: List[Int] } because the type args align. Similarly, here we allow arbitrary outer because it is a runtime check.