Constructor not generated on 3.3.1

[He-Pin]

Compiler version

3.3.1
In pekko /Akka project

Minimized code

Sbt docs
+3.3.1
TestOnly ※ActorDocSpec※

I edited this on phone , sorry

"Xxxxx test " in {

new AnyRef {

  Class WatchActor extends Actor {
     ....
   }

  val ref = system.actorOf(Props(classOf[WatchActor], this))

}

}

Output

apache/pekko#1082

Expectation

Works as scala 2.12 and 2.13

The WatchActor is in a none static scope, so the first argument of it's constructor should be the type of the outter scope, but there is no matched constructors, seems just ignored the outter scope.

See Class.getDeclaredConstructors for more details.

[Roiocam]

reproduce script

A simple reproduce scala-cli script like this:

//> using scala 2.13.8
//> using dep "org.apache.pekko::pekko-actor:1.0.2"
import org.apache.pekko.actor.ActorSystem

val system = ActorSystem("Main")
println("test")
val ref = new AnyRef {
  import org.apache.pekko.actor.{ Actor, Props, Terminated }

  class WatchActor extends Actor {
    val child = context.actorOf(Props.empty, "child")
    context.watch(child)
    var lastSender = context.system.deadLetters

    def receive = {
      case "kill" =>
        context.stop(child)
        lastSender = sender()
      case Terminated(`child`) =>
        lastSender ! "finished"
    }
  }
  val victim = system.actorOf(Props(classOf[WatchActor], this))
  print(victim.path.toSerializationFormat)
}
Thread.sleep(1000)

Scala 3.3.1 result

> Scala-cli Test.sc
Compiling project (Scala 3.3.1, JVM)
Compiled project (Scala 3.3.1, JVM)
test
Exception in thread "main" java.lang.IllegalArgumentException: no matching constructor found on class Test$_$$anon$1$WatchActor for arguments [class Test$_$$anon$1]
        at org.apache.pekko.util.Reflect$.error$1(Reflect.scala:98)
        at org.apache.pekko.util.Reflect$.findConstructor(Reflect.scala:122)
        at org.apache.pekko.actor.ArgsReflectConstructor.<init>(IndirectActorProducer.scala:109)
        at org.apache.pekko.actor.IndirectActorProducer$.apply(IndirectActorProducer.scala:74)
        at org.apache.pekko.actor.Props.producer(Props.scala:141)
        at org.apache.pekko.actor.Props.<init>(Props.scala:154)
        at org.apache.pekko.actor.Props$.apply(Props.scala:123)
        at org.apache.pekko.actor.Props$.apply(Props.scala:96)
        at Test$_$$anon$1.<init>(Test.sc:23)
        at Test$_.<init>(Test.sc:25)
        at Test_sc$.script$lzyINIT1(Test.sc:41)
        at Test_sc$.script(Test.sc:41)
        at Test_sc$.main(Test.sc:45)
        at Test_sc.main(Test.sc)

Scala 2.13.8 result

> Scala-cli Test.sc
Compiling project (Scala 2.13.8, JVM)
Compiled project (Scala 2.13.8, JVM)
test
pekko://Main/user/$a#452614370

[He-Pin]

Thanks, @Roiocam , ping @dwijnand ~~

[sjrd]

It probably has a constructor that takes no argument, because it is effectively a local class, and it does not need the reference to the enclosing AnyRef anonymous class.

[dwijnand]

No, we need a reproducer with no dependencies, so we can have it as a test case.

[He-Pin]

The bug is: The actor WatchActor is inside a scope, so the first parameter should be the new AnyRef, but there is no matched constructor.

Returns a Constructor object that reflects the specified constructor of the class represented by this Class object. The parameterTypes parameter is an array of Class objects that identify the constructor's formal parameter types, in declared order. If this Class object represents an inner class declared in a non-static context, the formal parameter types include the explicit enclosing instance as the first parameter.

public Constructor<T> getDeclaredConstructor(Class<?>... parameterTypes)
        throws NoSuchMethodException, SecurityException

I'm on windows and the bug of scala-cli prevents me to test it :(
Scala 3 not generate the constructor which should be the the formal parameter types include the explicit enclosing instance as the first parameter.

If this Class object represents an inner class declared in a non-static context, the formal parameter types include the explicit enclosing instance as the first parameter.

[He-Pin]

@dwijnand I think you know Akka/Pekko better than me, would you like to help out, thanks.

[dwijnand]

object helper {
  def test(cls: Class[?]) = println(cls.getDeclaredConstructors.toList)
}
import helper.test

object T1 { class C1; test(classOf[C1]) }
object T2 { new AnyRef { class C2; test(classOf[C2]) } }
object T3 { def t3(): Unit = { class C3; test(classOf[C3]) } }
object T4 { def t4(): Unit = new AnyRef { class C4; test(classOf[C4]) } }

class T5 { class C5; test(classOf[C5]) }
class T6 { new AnyRef { class C6; test(classOf[C6]) } }
class T7 { def t7(): Unit = { class C7; test(classOf[C7]) } }
class T8 { def t8(): Unit = new AnyRef { class C8; test(classOf[C8]) } }

object Test {
  def main(args: Array[String]): Unit = {
    T1.toString
    T2.toString
    T3.t3()
    T4.t4()
    new T5().toString
    new T6().toString
    new T7().t7()
    new T8().t8()
  }
}

$ scalac -2.13.12 Test.scala && scala -2.13.12 Test
List(public T1$C1())
List(public T2$$anon$1$C2(T2$$anon$1))
List(public T3$C3$1())
List(public T4$$anon$2$C4(T4$$anon$2))
List(public T5$C5(T5))
List(public T6$$anon$3$C6(T6$$anon$3))
List(public T7$C7$1(T7))
List(public T8$$anon$4$C8(T8$$anon$4))
$
$ scalac -3.3.1 Test.scala && scala -3.3.1 Test
List(public T1$C1())
List(public T2$$anon$1$C2(T2$$anon$1))
List(public T3$C3$1(T3$))
List(public T4$$anon$2$C4(T4$$anon$2))
List(public T5$C5(T5))
List(public T6$$anon$3$C6())
List(public T7$C7$1())
List(public T8$$anon$4$C8())

The ActorDocSpec case is represented by T6.

So it looks like Scala 3 loses the enclosing class as soon as the class is within an anonymous class or a method (because both are unnameable?), provided we're within a class. When we're in an object, we keep the enclosing anonymous class, when we have one, however we also keep the enclosing module class, when we're in an object method, which I think is also wrong.

[mbovel]

This issue was picked for the Issue Spree of February 27th, 2024. @dwijnand, @mbovel and @noti0na1 will be working on it. If you have any insight into the issue or guidance on how to fix it, please leave it here.

[mbovel]

It probably has a constructor that takes no argument, because it is effectively a local class, and it does not need the reference to the enclosing AnyRef anonymous class.

So is this really a bug? Must the constructor of the inner class always take an outer class instance even if it doesn't use it?

[mbovel]

Let tests/run/19569.scala contain:

class T:
  def t() =
    new AnyRef:
      val x = 0
      class C:
        def g = x

Then scalac -Xprint:flatten tests/run/19569.scala shows:

  class T$$anon$1$C extends Object {
    def <init>($outer: Object {...}): Unit =
      {
        if $outer.eq(null) then throw new NullPointerException() else ()
        T$$anon$1$C.this.$outer = $outer
        super()
        ()
      }
    def g(): Int = this.$outer.x()
    private val $outer: Object {...}
    final def T$_$$anon$C$$$outer(): Object {...} = T$$anon$1$C.this.$outer
  }

Now let tests/run/19569.scala contain (such that C doesn't need the outer anonymous class anymore):

class T:
  def t() =
    new AnyRef:
      class C:
        val x = 0
        def g = x

Then scalac -Xprint:flatten tests/run/19569.scala shows (no constructor argument anymore):

 class T$$anon$1$C extends Object {
    def <init>(): Unit =
      {
        super()
        this.x = 0
        ()
      }
    private val x: Int
    def x(): Int = this.x
    def g(): Int = this.x()
  }

[odersky]

That's very much of a feature in Scala 3. Unnecessary outer pointers are a source of memory leaks. Scala 3 does a much better job at eliminating outer pointers that don't affect semantics (ignoring reflection). And we don't want to go back.

[sjrd]

I agree. There's no point in generating useless outer pointers, even as constructor parameters, for local classes.

Even the JavaDoc reflection spec does not apply here, because this is a local class, not an inner class.

[dwijnand]

We can fix the T3 example above. And we can describe how Akka/Pekko can handle these classes in their reflection utilities.

[dwijnand]

Even the JavaDoc reflection spec does not apply here, because this is a local class, not an inner class.

I don't think that's right, because a local class is an inner class, from jls-8.1.3:

An inner class is one of the following:

• a member class that is not explicitly or implicitly static (§8.5)
• a local class that is not implicitly static (§14.3)
• an anonymous class (§15.9.5)

[mbovel]

Would anyone have insights on what hasLocalInstantiation means and do exactly? A comment at call site says !hasLocalInstantiation(cls) // needs outer because we might not know whether outer is referenced or not; what are the cases in which we can't know if outer will be referenced or not?

In particular, hasLocalInstantiation returns true in the case of C3, but not C7, why?

// static?
class T3  {
  // non-static
  def t3(): Unit = {
    // non-static
    class C3
  }
}

// static
object T7 {
  // static
  def t7(): Unit = {
    // non-static
    class C7
  }
}

Shouldn't hasLocalInstantiation return true for any class defined in a method?

[mbovel]

Tentative:

diff --git a/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala b/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala
index f57595293a..5fd506c91d 100644
--- a/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala
+++ b/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala
@@ -227,7 +227,7 @@ object ExplicitOuter {
   private def hasLocalInstantiation(cls: ClassSymbol)(using Context): Boolean =
     // Modules are normally locally instantiated, except if they are declared in a trait,
     // in which case they will be instantiated in the classes that mix in the trait.
-    cls.owner.ownersIterator.takeWhile(!_.isStatic).exists(_.isTerm)
+    cls.owner.ownersIterator.exists(o => o.is(Method) || o.isAnonymousClass)
     || cls.is(Private, butNot = Module)
     || cls.is(Module) && !cls.owner.is(Trait)

[odersky]

I think it should be

cls.owner.ownersIterator.takeWhile(!_.isStaticOwner).exists(_.isTerm)

[mbovel]

Ah cool, seems to work! It allows removing outer pointers for all Dale's examples (#19569 (comment)) except T5.

[dwijnand]

Indeed, isTerm works for the anonymous class too because sym = value <local T6> cls = anonymous class Object {...}, the anonymous class is within a local term value.

[mbovel]

By the way, we were wondering, are there symbols for which !o.isStatic && o.isTerm that are neither methods or anonymous classes (o.is(Method) || o.isAnonymousClass)?

[mbovel]

#19803 opened.

[dwijnand]

Going back to the Akka/Pekko use-case:

"xxxxx test" in {
  new AnyRef {
    class WatchActor extends Actor {
      ...
    }
    val ref = system.actorOf(Props(classOf[WatchActor], this))
  }
}

The second argument to Props defines the constructor arguments. In order to make this code cross-compatible between Scala 2 and Scala 3, Reflect.findConstructor will have to somehow consider if the constructor lacks the outer pointer parameter, so should drop the outer-pointer argument.