scala/scala3

Stack overflow when summoning `Mirror` for local class

Opened this issue · 10 comments

mrdziuban commented

Compiler version

3.3.3, 3.4.2, and the latest nightly 3.5.0-RC1-bin-20240514-7c9aae3-NIGHTLY

Minimized code

https://scastie.scala-lang.org/mrdziuban/6oBpwuegRvOjFsuhXKbfNA

def test(): Unit = {
  case class A()
  object A {
    val mirror = summon[scala.deriving.Mirror.ProductOf[A]]
  }
  println(A.mirror)
}

test()

Output

java.lang.StackOverflowError
	at Playground$A$3$.<init>(main.scala:5)
	at Playground$.A$lzyINIT1$1(main.scala:5)
	at Playground$.Playground$$$_$A$2(main.scala:5)
	at Playground$A$3$.<init>(main.scala:6)
	at Playground$.A$lzyINIT1$1(main.scala:5)
	at Playground$.Playground$$$_$A$2(main.scala:5)
	at Playground$A$3$.<init>(main.scala:6)
	at Playground$.A$lzyINIT1$1(main.scala:5)
	at Playground$.Playground$$$_$A$2(main.scala:5)
	at Playground$A$3$.<init>(main.scala:6)
	...

Expectation

Summoning the Mirror should work and not cause an error

bishabosha commented

the reason this happens is because you are initialising A in an infinite loop, the Mirror of class A is object A, so you are trying to store A in a field in its own body.

exactly like you did this:

def foo() =
  object Foo:
    val mirror = Foo
  println(Foo.mirror)

foo()
mrdziuban commented

I see, thanks for explaining @bishabosha. Why does it work for non-local classes though?

bishabosha commented

the solution here could be a linter that checks for these kinds of cases. (maybe @liufengyun knows)?

mrdziuban commented

For context, this came up in circe/circe#2263. It seems like disallowing cases like this would also mean derivation would no longer be possible, which feels less than ideal.

bishabosha commented

can use a lazy val?

mrdziuban commented

Yeah using a lazy val works as a workaround but in the context of derivation there's no way to make sure all downstream users do so

bishabosha commented

well is it possible to delay the evaluation of the mirror in the circe deriving code since you are retaining the mirror?

mrdziuban commented

That sounds promising, but I've tried a number of things and can't get it working. The only fix I've found so far is twofold:

  1. Mark the mirror param here as inline -- https://github.com/circe/circe/blob/v0.14.7/modules/core/shared/src/main/scala-3/io/circe/derivation/ConfiguredDecoder.scala#L222
  2. Construct the ConfiguredDecoder in the inline method rather than proxying to ConfiguredDecoder.of

Both of these have their own issues though:

  1. This requires downgrading Scala to 3.2.2, which in turn requires downgrading sbt and cats. In Scala 3.3 and later, the inline mirror results in errors like this: 
        -- [E083] Type Error: /Users/matt/circe/modules/core/shared/src/main/scala-3/io/circe/derivation/ConfiguredDecoder.scala:223:39
223 |    ConfiguredDecoder.of[A](constValue[mirror.MirroredLabel], decoders[A], summonLabels[mirror.MirroredElemLabels])
    |                                       ^^^^^^
    |(mirror : scala.deriving.Mirror.Of[A]) is not a valid type prefix, since it is not an immutable path
  2. This was purposely changed recently to avoid generating a new class file per call to derived -- circe/circe#2230

Here's my WIP branch in case you see anything I might be missing: circe/circe@series/0.14.x...mblink:fix-2263

mrdziuban commented

I'm far from an expert in interpreting how this might be related, but in case it helps someone else here's the output and diff of compiling a top-level vs. local class with -Xprint:genBCode

top-level-class-output.txt
local-class-output.txt

// top-level-class.scala
case object A {
  val mirror = summon[scala.deriving.Mirror.ProductOf[A.type]]
}
// local-class.scala
def test(): Unit = {
  case object A {
    val mirror = summon[scala.deriving.Mirror.ProductOf[A.type]]
  }
}

I can follow why there's a stack overflow with the local version -- during the initialization of the lazy val, the class constructor calls rs$line$1$$$_$A$1, which calls A$lzyINIT1$1, which calls the class constructor, etc.

Other notable differences (to me at least) are:

  1. case object A is represented as a module case class in the top-level version as opposed to a case class in the local version
  2. The class constructor takes a LazyRef representing an instance of the class itself
  3. val mirror is represented as a private <static> val in the top-level version as opposed to a private val in the local version
diff --git a/Users/matt/Desktop/top-level-class-output.txt b/Users/matt/Desktop/local-class-output.txt
index c8c1fa23f6b..906c223349f 100644
--- a/Users/matt/Desktop/top-level-class-output.txt
+++ b/Users/matt/Desktop/local-class-output.txt
@@ -7,17 +7,33 @@ package <empty> {
       }
     private def writeReplace(): Object =
       new scala.runtime.ModuleSerializationProxy(classOf[rs$line$1])
-    final lazy module case <static> val A: rs$line$1$A = new rs$line$1$A()
+    def test(): Unit =
+      {
+        lazy var A$lzy1: scala.runtime.LazyRef = new scala.runtime.LazyRef()
+        ()
+      }
+    private final def A$lzyINIT1$1(A$lzy1$1: scala.runtime.LazyRef):
+      rs$line$1$A$2 =
+      A$lzy1$1.synchronized[rs$line$1$A$2](
+        (if A$lzy1$1.initialized() then A$lzy1$1.value() else
+          A$lzy1$1.initialize(new rs$line$1$A$2(A$lzy1$1))).asInstanceOf[
+          rs$line$1$A$2]
+      )
+    final lazy case def rs$line$1$$$_$A$1(A$lzy1$2: scala.runtime.LazyRef):
+      rs$line$1$A$2 =
+      (if A$lzy1$2.initialized() then A$lzy1$2.value() else
+        this.A$lzyINIT1$1(A$lzy1$2)).asInstanceOf[rs$line$1$A$2]
   }
-  final module case class rs$line$1$A extends Object, Product,
+  private final case class rs$line$1$A$2 extends Object, Product,
     java.io.Serializable, scala.deriving.Mirror.Mirror$Singleton {
-    def <init>(): Unit =
+    def <init>(A$lzy1$3: scala.runtime.LazyRef): Unit =
       {
         super()
-        mirror =
+        this.mirror =
           {
             val x$proxy1: scala.deriving.Mirror.Mirror$Singleton =
-              A:scala.deriving.Mirror.Mirror$Singleton
+              rs$line$1.this.rs$line$1$$$_$A$1(A$lzy1$3):
+                scala.deriving.Mirror.Mirror$Singleton
             x$proxy1
           }
         ()
@@ -28,12 +44,10 @@ package <empty> {
       super[Product].productElementNames()
     def fromProduct(p: Product): scala.deriving.Mirror.Mirror$Singleton =
       super[Singleton].fromProduct(p)
-    private def writeReplace(): Object =
-      new scala.runtime.ModuleSerializationProxy(classOf[rs$line$1$A])
     override def hashCode(): Int = 65
     override def toString(): String = "A"
     override def canEqual(that: Object): Boolean =
-      that.isInstanceOf[rs$line$1$A]
+      that.isInstanceOf[rs$line$1$A$2]
     override def productArity(): Int = 0
     override def productPrefix(): String = "A"
     override def productElement(n: Int): Object =
@@ -48,8 +62,8 @@ package <empty> {
           case val x2: Int = n
           throw new IndexOutOfBoundsException(Int.box(n).toString())
         }
-    private <static> val mirror: scala.deriving.Mirror.Mirror$Singleton
-    def mirror(): scala.deriving.Mirror.Mirror$Singleton = mirror
+    private val mirror: scala.deriving.Mirror.Mirror$Singleton
+    def mirror(): scala.deriving.Mirror.Mirror$Singleton = this.mirror
     def fromProduct(p: Product): Object = this.fromProduct(p)
   }
   final lazy module val rs$line$1: rs$line$1 = new rs$line$1()
liufengyun commented

the solution here could be a linter that checks for these kinds of cases. (maybe @liufengyun knows)?

Non-termination is not captured by the initialization checker, as we know accurately checking non-termination is impossible.

To soundly check non-termination, the checker would report many false positives, which will be annoying.

To address the problem above and related problems with simple non-termination, it seems an unsound check of blatant non-terminations that only report true positives will be useful in practice.

/cc: @olhotak @EnzeXing