Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Case Class Factory Constructor Inlining Fails to Invoke Companion Constructor #5304

Open
scabug opened this issue Dec 13, 2011 · 12 comments
Open

Comments

@scabug
Copy link

scabug commented Dec 13, 2011

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
Copy link
Author

scabug commented Dec 13, 2011

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

@scabug
Copy link
Author

scabug commented May 2, 2012

@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
Copy link
Author

scabug commented May 8, 2012

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

@scabug
Copy link
Author

scabug commented May 8, 2012

@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
Copy link
Author

scabug commented Jun 2, 2012

@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
Copy link
Author

scabug commented Jun 5, 2012

@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
Copy link
Author

scabug commented Jan 13, 2013

@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
Copy link
Author

scabug commented Jan 26, 2013

@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
Copy link
Author

scabug commented Jan 26, 2013

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

@scabug
Copy link
Author

scabug commented Jul 30, 2013

@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);
      ()
    }
  }
}

@SethTisue
Copy link
Member

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

@SethTisue
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants