bug icon indicating copy to clipboard operation
bug copied to clipboard

Case Class Factory Constructor Inlining Fails to Invoke Companion Constructor

Open scabug opened this issue 13 years ago • 14 comments

See the following paste. It appears that the inlining of case class constructors/deconstructors fails to invoke the constructor for the companion object, which is counter to the specification.

Welcome to Scala version 2.10.0.rdev-4009-2011-12-13-g6912ff8 (Java HotSpot(TM) 64-Bit Server VM, Java 1.6.0_29).
Type in expressions to have them evaluated.
Type :help for more information.
scala> :paste
// Entering paste mode (ctrl-D to finish)

case class Test(a: Int)
object Test {
  println("here")
}

// Exiting paste mode, now interpreting.

defined class Test
defined module Test

scala> val Test(a) = Test(42)
a: Int = 42

scala> Test
here
res0: Test.type = Test$@78da5318

scabug avatar Dec 13 '11 20:12 scabug

Imported From: https://issues.scala-lang.org/browse/SI-5304?orig=1 Reporter: @djspiewak Affected Versions: 2.10.0 See #4859

scabug avatar Dec 13 '11 20:12 scabug

@lrytz said: Spec 5.4 says

bq. The new m$cls constructor is evaluated not at the point of the object definition, but is instead evaluated the first time m is dereferenced during execution of the program (which might be never at all).

scabug avatar May 02 '12 15:05 scabug

@adriaanm said: companion objects may have side effects, caveat inliner

scabug avatar May 08 '12 15:05 scabug

@som-snytt said: Cheap workaround:

case class Shu(value: Int) { Shu }

Less cheap:

case class Ma(value: Int)(implicit dummy: Ma.type = Ma)

I noticed that the transform is done in RefChecks, and it's so simple and clean that you want to spec it and say Test(42) is a sweetened new Test(42).

But I also see that simple sugars are unhealthy:

case class Zi private (value: Int)
object Test { 
  //val z2 = new Zi(19) // obviously not
  val z = Zi(17)  // error: constructor Zi in class Zi cannot be accessed in object Test
} 

I see I can't supply a custom non-synthetic apply to short-circuit the optimization either; the spec does say that you can roll your own copy method.

scabug avatar May 08 '12 18:05 scabug

@magarciaEPFL said: I've looked at the bytecode emitted with and without -optimize, for each of the scenarios below:

  • Test is a case class Test(42) Outcome: doesn't invoke the companion object's constructor
  • Test is a non-case class new Test(42) Outcome: doesn't invoke the companion object's constructor

Now, let's turn this around. Instead of having me show the above is per-spec, can you provide a counter-argument? Which place in the spec mentions the above should behave differently?

scabug avatar Jun 02 '12 13:06 scabug

@djspiewak said: Test(42) should be semantically equivalent to val t = Test; t.apply(42). Or, equivalently, val t = Test; val x = t.x; t.apply(42). If x is free of side-effects, then all of these should be semantically equivalent. However, the first one will fail to invoke the companion object constructor, while the third (and I believe the second) will.

scabug avatar Jun 05 '12 14:06 scabug

@retronym said: My proposed fix for #4859 stopped just short of fixing this problem, but it shows all the places that need to be touched when we decide to do so.

scabug avatar Jan 13 '13 14:01 scabug

@retronym said: I discussed this with Martin recently, his thoughts:

  • Accessing a nested Java static classes doesn't run the static initializer of the enclosing class
  • People might be surprised by a performance difference between nested and top level modules
  • But, the arguments for forcing initialization do make sense.
  • We might investigate a scheme in which the constructor of a nested module initializes it's enclosing module. This would give us the desired semantics without the penalty of the outer module load at every access of the nested module.

scabug avatar Jan 26 '13 14:01 scabug

@retronym said: Rescheduling for 2.11, I don't want to change the semantics for a minor release.

scabug avatar Jan 26 '13 16:01 scabug

@retronym said: Another example. This stems from use of isExprSafeToInline in Erasure#unbox1.

topic/typer-crash-cleanup /code/scala tail sandbox/{t1,c}.scala
==> sandbox/t1.scala <==
object T { println("!!!"); val x: Unit = () }
==> sandbox/c.scala <==
object C {
  def main(args: Array[String]) {
    Some[Unit](T.x)
  }
}
topic/typer-crash-cleanup /code/scala qbin/scalac -d sandbox -Xprint:erasure sandbox/{t1,c}.scala
[[syntax trees at end of                   erasure]] // t1.scala
package <empty> {
  object T extends Object {
    def <init>(): T.type = {
      T.super.<init>();
      ()
    };
    private[this] val x: scala.runtime.BoxedUnit = scala.runtime.BoxedUnit.UNIT;
    <stable> <accessor> def x(): Unit = ()
  }
}

 // c.scala
package <empty> {
  object C extends Object {
    def <init>(): C.type = {
      C.super.<init>();
      ()
    };
    def main(args: Array[String]): Unit = {
      new Some(scala.runtime.BoxedUnit.UNIT);
      ()
    }
  }
}

scabug avatar Jul 30 '13 13:07 scabug

another, related example in #8666 (which I'm closing, let's consolidate here)

SethTisue avatar Mar 01 '18 21:03 SethTisue

another, related example in #9115 (which I'm closing, let's consolidate here)

SethTisue avatar Feb 16 '24 23:02 SethTisue

This use case is fixed in dotty (constructor proxy) but 9115 is not (nested static class init).

som-snytt avatar Aug 27 '24 08:08 som-snytt

I've reopened #9115.

SethTisue avatar Aug 29 '24 11:08 SethTisue