Defining a given referring to a given inside a trait crashes

[travisbrown]

minimized code

trait Foo[A]
trait Bar[A] { def foo: Foo[A] }

trait Qux[A] given (A: Bar[A]) {
  given as Foo[A] = A.foo
}

Giving it an explicit name doesn't help.

Stack trace

java.lang.AssertionError: assertion failed: method Foo_A_given$_cache while compiling foo.scala
Exception in thread "main" java.lang.AssertionError: assertion failed: method Foo_A_given$_cache
        at dotty.DottyPredef$.assertFail(DottyPredef.scala:16)
        at dotty.tools.dotc.ast.tpd$TreeOps$.$anonfun$2(tpd.scala:969)
        at dotty.tools.dotc.core.Symbols$Symbol.orElse(Symbols.scala:600)
        at dotty.tools.dotc.ast.tpd$TreeOps$.becomes$extension(tpd.scala:971)
        at dotty.tools.dotc.transform.Getters.transformAssign(Getters.scala:88)
        at dotty.tools.dotc.transform.MegaPhase.goAssign(MegaPhase.scala:686)
        at dotty.tools.dotc.transform.MegaPhase.transformUnnamed$1(MegaPhase.scala:325)
        at dotty.tools.dotc.transform.MegaPhase.transformTree(MegaPhase.scala:396)
        at dotty.tools.dotc.transform.MegaPhase.transformUnnamed$1(MegaPhase.scala:297)
        at dotty.tools.dotc.transform.MegaPhase.transformTree(MegaPhase.scala:396)
        at dotty.tools.dotc.transform.MegaPhase.transformStat$2(MegaPhase.scala:406)
        at dotty.tools.dotc.transform.MegaPhase.$anonfun$1(MegaPhase.scala:409)
        at scala.collection.immutable.List.mapConserve(List.scala:179)
        at dotty.tools.dotc.transform.MegaPhase.transformStats(MegaPhase.scala:409)
        at dotty.tools.dotc.transform.MegaPhase.transformUnnamed$1(MegaPhase.scala:286)
        at dotty.tools.dotc.transform.MegaPhase.transformTree(MegaPhase.scala:396)
        at dotty.tools.dotc.transform.MegaPhase.mapDefDef$1(MegaPhase.scala:245)
        at dotty.tools.dotc.transform.MegaPhase.transformNamed$1(MegaPhase.scala:248)
        at dotty.tools.dotc.transform.MegaPhase.transformTree(MegaPhase.scala:394)
        at dotty.tools.dotc.transform.MegaPhase.transformStat$2(MegaPhase.scala:404)
        at dotty.tools.dotc.transform.MegaPhase.$anonfun$1(MegaPhase.scala:409)
        at scala.collection.immutable.List.mapConserve(List.scala:179)
        at dotty.tools.dotc.transform.MegaPhase.transformStats(MegaPhase.scala:409)
        at dotty.tools.dotc.transform.MegaPhase.transformUnnamed$1(MegaPhase.scala:339)
        at dotty.tools.dotc.transform.MegaPhase.transformTree(MegaPhase.scala:396)
        at dotty.tools.dotc.transform.MegaPhase.transformNamed$1(MegaPhase.scala:251)
        at dotty.tools.dotc.transform.MegaPhase.transformTree(MegaPhase.scala:394)
        at dotty.tools.dotc.transform.MegaPhase.transformStat$2(MegaPhase.scala:404)
        at dotty.tools.dotc.transform.MegaPhase.$anonfun$1(MegaPhase.scala:409)
        at scala.collection.immutable.List.mapConserve(List.scala:179)
        at dotty.tools.dotc.transform.MegaPhase.transformStats(MegaPhase.scala:409)
        at dotty.tools.dotc.transform.MegaPhase.mapPackage$1(MegaPhase.scala:356)
        at dotty.tools.dotc.transform.MegaPhase.transformUnnamed$1(MegaPhase.scala:359)
        at dotty.tools.dotc.transform.MegaPhase.transformTree(MegaPhase.scala:396)
        at dotty.tools.dotc.transform.MegaPhase.transformUnit(MegaPhase.scala:415)
        at dotty.tools.dotc.transform.MegaPhase.run(MegaPhase.scala:427)
        at dotty.tools.dotc.core.Phases$Phase.runOn$$anonfun$1(Phases.scala:316)
        at scala.collection.immutable.List.map(List.scala:286)
        at dotty.tools.dotc.core.Phases$Phase.runOn(Phases.scala:318)
        at dotty.tools.dotc.Run.runPhases$4$$anonfun$4(Run.scala:158)
        at dotty.runtime.function.JProcedure1.apply(JProcedure1.java:15)
        at dotty.runtime.function.JProcedure1.apply(JProcedure1.java:10)
        at scala.collection.IndexedSeqOptimized.foreach(IndexedSeqOptimized.scala:36)
        at scala.collection.IndexedSeqOptimized.foreach$(IndexedSeqOptimized.scala:33)
        at scala.collection.mutable.ArrayOps$ofRef.foreach(ArrayOps.scala:198)
        at dotty.tools.dotc.Run.runPhases$5(Run.scala:170)
        at dotty.tools.dotc.Run.compileUnits$$anonfun$1(Run.scala:178)
        at dotty.runtime.function.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:12)
        at dotty.tools.dotc.util.Stats$.maybeMonitored(Stats.scala:102)
        at dotty.tools.dotc.Run.compileUnits(Run.scala:185)
        at dotty.tools.dotc.Run.compileSources(Run.scala:120)
        at dotty.tools.dotc.Run.compile(Run.scala:104)
        at dotty.tools.dotc.Driver.doCompile(Driver.scala:34)
        at dotty.tools.dotc.Driver.process(Driver.scala:172)
        at dotty.tools.dotc.Driver.process(Driver.scala:141)
        at dotty.tools.dotc.Driver.process(Driver.scala:153)
        at dotty.tools.dotc.Driver.main(Driver.scala:180)
        at dotty.tools.dotc.Main.main(Main.scala)

[travisbrown]

Oh, actually this can be minimized:

trait Foo[A]

trait Qux {
  given as Foo[Int] = new Foo[Int] {}
}

But this works just fine:

trait Foo[A]


trait Qux {
  val x = new Foo[Int] {}

  given as Foo[Int] = x
}

[smarter]

The CacheAliasImplicits phase rewrites implicit/given definitions: https://github.com/lampepfl/dotty/blob/f87554b6161967e5ea76c88997579599ff33b7d8/compiler/src/dotty/tools/dotc/transform/CacheAliasImplicits.scala#L26-L51

Here in particular, we get:

trait Qux {
  private[this] var Foo_Int_given$_cache: TC = null
  implicit def Foo_Int_given: Foo[Int] = {
    if (Foo_Int_given$_cache == null)
      Foo_Int_given$_cache = new Foo[Int] {}
    Foo_Int_given$_cache
  }
}

Which then cause a crash in the Getters phase, because a var in a trait is supposed to have a corresponding setter method, but this setter method is added much earlier than CacheAliasImplicits, during desugaring: https://github.com/lampepfl/dotty/blob/f87554b6161967e5ea76c88997579599ff33b7d8/compiler/src/dotty/tools/dotc/ast/Desugar.scala#L147-L152

This is fixable through some refactoring, but IMHO we should rethink this whole caching strategy, since I don't think we've considered all its consequences yet (e.g. adding a var to a trait is a binary-breaking change, so now adding a given may also be a binary-breaking change), not to mention than even though the documentation says that this isn't thread-safe, people are likely to assume it is anyway. This is one of the thing that should have been discussed in https://contributors.scala-lang.org/t/proposal-to-add-implied-instances-to-the-language/3070/1 but got lost in the never-ending quest for the perfect keyword unfortunately.

[eed3si9n]

I ran into it as well. Using abstract class for now.