Imported From: https://issues.scala-lang.org/browse/SI-9931?orig=1
Reporter: Alistair Johnson (alistair.johnson-at-johnsonusm.com)
Affected Versions: 2.12.0-RC1

@SethTisue said:
isolated and minimized:

trait Compute[A] {
  type Start
  val start: Compute[Start]
}

object Test {
  def foo[A](c: Compute[A]): Unit =
    c match {
      case c: Compute[A] =>
        new Compute[A] {
          type Start = c.Start
          val start = c.start
        }
    }
}

Perhaps it can be minimized further, but I got stuck here. In particular, I did not succeed in getting rid of the pattern match.

This almost certainly has to do with the changes in type inference for valdefs that came in scala/scala#5141. As evidence, compiling with 2.12.0-M5 makes the error go away, as does compiling with -Xsource:2.11.

@SethTisue said:
Another way to make this compile is:

-      case c: Compute[A] =>
+      case d: Compute[A] =>
+        val c = d

which certainly makes it seem like it's a pattern-matcher issue.

@SethTisue said (edited on Sep 21, 2016 10:21:46 PM UTC):
Here's a variant minimized form (eliminating a type parameter, but introducing a type variable):

trait Compute[A] {
  type Start
  val start: Compute[Start]
}

object Test {
  def foo(c: Any): Unit =
    c match {
      case c: Compute[a] =>
        new Compute[a] {
          type Start = c.Start
          val start = c.start
        }
    }
}

It's not that interestingly different, because in both versions, the type parameter on Compute isn't actually used anywhere. Regardless, as far as I can tell, you do need to have a type parameter in order to trigger the issue. (Throwing away the type member instead makes the issue vanish, too.)

@SethTisue said:
Most of the commits at https://github.com/scala/scala/pull/5141/commits are red so I can't quickly bisect to a particular commit without actually building those Scala versions. But that's definitely the PR it regressed in. The code compiles in aab103e but by f1cbe8a the error has started.

@adriaanm said:
Thanks for minimizing. This makes a pretty strong case that it's the val signature inference changes. I'm still away from my computer until Monday, but you could try with a def instead of a val and then it should behave the same across versions. Assigning a result type so that the inherited one is overridden wil be needed from now on (as in dotty).

@SethTisue said:

you could try with a def instead of a val and then it should behave the same across versions

With "def" you get the error in both 2.11.8 and 2.12.0-RC1.

Dotty accepts both the "def" and "val" versions, so this isn't a case where we can say "we're like Dotty now".

@adriaanm said:
Fascinating! As you know, it compiles ok on 2.11.8,.... except when you add -Xprint:typer! (In the example below I added -Ydebug for a better error message, but it's otherwise not involved.)

This indicates to me it used to compile due to a latent bug in the old handling of fields. It still doesn't explain why it compiles on Dotty. Still investigating that avenue.

➜  scala git:(2.12.0) ~/scala/scala-2.11.8/bin/scalac /tmp/Eval.scala -Xprint:typer -Ydebug

[...]

/tmp/Eval.scala:7: error: overriding value start in class Compute of type scala.this.Function0[<empty>.this.Eval[$anon.this.Start]];
 value start  has incompatible type;
 found   : => scala.this.Function0[<empty>.this.Eval[x2.Start]]
 required: => scala.this.Function0[<empty>.this.Eval[Compute.this.Start]]
          val start = c.start
              ^
one error found

sealed abstract class Eval[+A] {
  def flatMap[B](f: A => Eval[B]): Eval[B] =
    this match {
      case c: Eval.Compute[A] =>
        new Eval.Compute[B] {
          type Start = c.Start
          val start = c.start
        }
    }
}

object Eval {
  sealed abstract class Compute[A] extends Eval[A] {
    type Start
    val start: () => Eval[Start]
  }
}

@adriaanm said:
So, that confirms your initial assessment: that x2 is a local variable that's synthesized by the pattern matcher. As you showed, when you manually introduce a local variable, the bug goes away, which points to inconsistent substitution in the pattern matcher phase. Given all this, I'm inclined to push this to 2.12.1, as it's not really a new bug in 2.12.0, and it has a workaround.

Alistair Johnson (alistair.johnson-at-johnsonusm.com) said:
OK that this is pushed to 2.12.1, thanks for thorough investigation.

So just doing some house cleaning: I have added a workaround in this cats PR - could someone cast a quick eye that the fix is the best for now, please? Then I can leave this for a while...

Thx - Alistair

@SethTisue said:
I would suggest you comment your fix, with a link to this ticket, as it isn't obvious why the type annotations would be necessary. Otherwise it seems fine to me. Except that perhaps you might prefer the "val c = d" style fix from my comment above?

Alistair Johnson (alistair.johnson-at-johnsonusm.com) said:
Awesome, just what I was after - I'll add on next push, when the code is reviewed ;)

This is a cache invalidation bug in TypeRef.normalized.

Exception in thread "main" java.lang.AssertionError: assertion failed: (Base.this.Start,c.Start,x2.Start)

diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala
index 97aea13c3b..ff1fe95311 100644
--- a/src/reflect/scala/reflect/internal/Types.scala
+++ b/src/reflect/scala/reflect/internal/Types.scala
@@ -2249 +2249 @@ trait Types
-        if (normalized eq null)
+        if (normalized eq null) {
@@ -2250,0 +2251,3 @@ trait Types
+        } else {
+          assert(normalized =:= normalizeImpl, (this, normalized, normalizeImpl))
+        }
@@ -2251,0 +2255 @@ trait Types
+

We have some code that tried to invalidate caches after substitution, but this example is a step ahead. Maybe we need to rethink this, perhaps we could disable these caches in symbols and typerefs that are local, which is where all the problems arise.

WIP for disabling caching for types formed over local symbols: https://github.com/scala/scala/compare/2.12.x...retronym:ticket/9931?expand=1

I've moved this from the 2.12.5 milestone to Backlog, but also: bump!, in case anyone wants to resume work on this.

Minimization works in 2.13.0, test in PR but I did not investigate.

@som-snytt thanks for all the triage just lately ❤️

I'm just the right age for triage. I was looking for an old ticket and found others instead.