Pattern matcher needs to check for outer

[odersky]

When testing against an inner class, the pattern matcher needs to test outer pointers. Consider the following program:

class Outer {

  case class Inner()

  val inner: Inner = new Inner

  def checkInstance(o: Outer) =
    o.inner.isInstanceOf[this.Inner]

  def checkPattern1(i: Any) =
    i match {
      case _: Inner => true
      case _ => false
    }

  def checkPattern2(i: Any) =
    i match {
      case Inner() => true
      case _ => false
    }

  def checkEquals(o: Outer) =
    o.inner == inner
}

object Test {

  def main(args: Array[String]) = {
    val o1 = new Outer
    val o2 = new Outer
    assert(o1.checkInstance(o2)) // ok
    assert(!o1.checkPattern1(o2.inner)) // ok under scalac, fails for dotc-compiled code
    assert(!o1.checkPattern2(o2.inner))  // ok under scalac, fails for dotc-compiled code
    assert(!o1.checkEquals(o2))  // ok under scalac, fails for dotc-compiled code
  }
}

We expect the isInstanceOf test in checkInstance to return true but the other three tests should return false. Yes, I know it's contorted, but these Scala rules were a compromise between
not losing performance and being precise. Essentially we accept that we lose precision in isInstanceOf because otherwise there would be no way to avoid the "outer test tax". But we demand that pattern matchers test outer pointers. Indeed you will find specific code doing that after patmat when you compile the code with scalac:

    if (x1.isInstanceOf[Outer.this.Inner].&&
    ((x1.asInstanceOf[Outer.this.Inner]: Outer.this.Inner).<outer>.eq(Outer.this)))

But such code is currently not emitted by dotc which is why the last three asserts fail.

The discrepancy is the reason why the semantic names PR fails the bootstrap.

[DarkDimius]

I'm having a look at this

[smarter]

The discrepancy is the reason why the semantic names PR fails the bootstrap.

Hah, that must have been pretty hard to find. How is this feature used for semantic names exactly?

[DarkDimius]

@smarter, Martin's design uses inner classes and pattern-matches over them.

[DarkDimius]

Have a fix, running full test suite to verify it.

[DarkDimius]

The bug has specifically to do with the outer being this.

[DarkDimius]

I've made a proper fix for this, unfortunatelly: it broke bootstrap

in https://github.com/lampepfl/dotty/blob/master/compiler/src/dotty/tools/io/ClassPath.scala#L245
we have a similar pattern match:

abstract class ClassPath {
 ...
 case class ClassRep(..){ ... }

 rep map {
          case x: ClassRep  => x
          case x            => throw new FatalError("Unexpected ClassRep '%s' found searching for name '%s'".format(x, name))
 }
}

For some reason, Scalac doesn't emmit an outer-class check here. @odersky do you know what's the difference between those two examples?

[odersky]

I think it's a bug in scalac. I played with it a little. First, with the code the map expands to:

    rep map ((x:ClassPath # ClassRep) => x match {
      case x: ClassRep  => x
      case x            => throw new FatalError("Unexpected ClassRep '%s' found searching for name '%s'".format(x, name))
    })

Here the outer check is omitted. But now change the parameter's type to Any:

    rep map ((x:Any) => x match {
      case x: ClassRep  => x
      case x            => throw new FatalError("Unexpected ClassRep '%s' found searching for name '%s'".format(x, name))
    })

The outer check reappears! This makes no sense. A selector of type ClassPath # ClassRep has exactly as much information about the outer pointer as a selector of type Any (i.e. none). So there should not be a difference in the generated code.

My advice would be to delete the whole map, which seems to have never done the right thing.

[lrytz]

Note that scalac omits the outer check for final nested classes

scala> class C { class I; final class J }
defined class C

scala> val c1, c2 = new C
c1: C = C@14f3c6fc
c2: C = C@3cd59ef5

scala> (new c1.I: C#I) match { case _: c2.I => 0; case _ => 1 }
res0: Int = 1

scala> (new c1.J: C#J) match { case _: c2.J => 0; case _ => 1 }
warning: there was one unchecked warning; for details, enable `:setting -unchecked' or `:replay -unchecked'
res1: Int = 0