Skip to content

Initialization of nested objects differs based on whether they are wrapped in a trait #21444

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

Open
lihaoyi opened this issue Aug 27, 2024 · 10 comments

Comments

@lihaoyi
Copy link
Contributor

lihaoyi commented Aug 27, 2024

Compiler version

3.5.0, same behavior in 2.13.14

Minimized code

The following code does not print Hello outer

object outer{
  println("Hello outer")
  object Foo
}
object Test{
  def main(args: Array[String]): Unit = println("Hello " + outer.Foo)
}

But if I wrap the object outer in a trait, it does print Hello outer

trait wrapper{
  object outer{
    println("Hello outer")
    object Foo
  }
}
object Test{
  def main(args: Array[String]): Unit = {
    println("Hello " + new wrapper{}.outer.Foo)
  }
}

Expectation

I would expect the initialization behavior of nested objects to be consistent regardless of whether or not they are wrapped in traits

@lihaoyi lihaoyi added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Aug 27, 2024
@SethTisue
Copy link
Member

SethTisue commented Aug 27, 2024

@lrytz @som-snytt does this ring a bell? I feel like there are old tickets on this . there’s scala/scala-dev#16 but it's more about the optimizer

there's also scala/bug#5304 (comment) , and also the linked tickets from there — maybe that's actually what I'm remembering 🤔

@lihaoyi
Copy link
Contributor Author

lihaoyi commented Aug 27, 2024

FWIW we have code in Mill that wraps things in classes just to work around this issue, which is how I noticed. Not the end of the world, but would be nice if i could eiminate the awkward wrapper class and still have proper initiaization behavior

https://github.com/com-lihaoyi/mill/blob/2f933877824a9c38a1ee9cdbca028212d535ef19/runner/src/mill/runner/MillBuildRootModule.scala#L384

@sjrd
Copy link
Member

sjrd commented Aug 27, 2024

IMO this is basically by design at this point. The behavior inside a trait (or class) is the one that makes the most sense, and also the one that could not be changed even if we wanted to.

But the behavior for static objects is more efficient, and there's bound to be a number of codebases that basically rely on that better efficiency. I know I have.

@lihaoyi
Copy link
Contributor Author

lihaoyi commented Aug 27, 2024

Some kind of wontfix is fine by me. Apart from performance, changing this now would seem like a pretty big breaking change semantically.

But if we're not going to fix it, should it at least be documented in the spec somewhere? It's super strange behavior after all, and even after playing around for a bit I'm still not totally sure what I should be expecting (e.g. doing val x = outer; x.Foo seems to initialize outer correctly)

@lrytz
Copy link
Member

lrytz commented Aug 27, 2024

Agree @sjrd. Just to confirm, changing final def isExprSafeToInline(tree: Tree): Boolean = false in Scala 2 changes the behavior, so it's not for some other ("unexpected") reason.

Also agree that it should be in the spec, like the difference between a reference to outer and outer appearing on a path.

@som-snytt
Copy link
Contributor

Spec says that an object is a lazy val, except for top-level. It should say more there.

We are lulled into complacency because we write top-level object from day one. object HelloWorld, object Test extends App. Maybe that will change with @main and enum. Top-level object is the odd duck.

Agree with Seth that there is too much history. I recently read an old Odersky comment on the same refrain about nested static classes not forcing. I have no idea where that was. Why are we having this conversation in 2024? Is it documentation, tooling, or implementation?

I think -Ysafe-init (or -Ysafe-init-global?) should warn me for outer in path may not be initialized. Or add syntax that lets me guarantee that behavior: Foo.safe.Bar. That is not weirder than locutions such as x.nn.

@olhotak
Copy link
Contributor

olhotak commented Aug 27, 2024

cc @liufengyun

@liufengyun
Copy link
Contributor

liufengyun commented Aug 27, 2024

While working on -Ysafe-init-global, we were surprised by some of the compiler optimizations that elide accesses of global objects --- e.g., in O.Inner and new O.C.

We have to teach the analysis to understand the optimization in order to support common Scala code patterns. Otherwise, we would have to report too many warnings about initialization cycles between outer static object and nested static objects.

Spec says that an object is a lazy val, except for top-level. It should say more there.

Maybe the spec can introduce the concept of static objects:

  • Top-level objects are static objects.
  • Objects directly nested inside static objects are also static objects.

Accessing an inner static object does not go through the outer static object. An expression new o.C does not access the static objects o if C is defined directly inside o (Related #20354). The rules are still simple and consistent.

The semantics of none-static objects is exactly as its desugaring to lazy val.

I think -Ysafe-init (or -Ysafe-init-global?) should warn me for outer in path may not be initialized.

Given that the initialization-time between the outer static object and inner static object are not correlated, such a warning should always be emitted, which makes it less useful.

@lihaoyi
Copy link
Contributor Author

lihaoyi commented Aug 28, 2024

Out of curiousity, does this behavior lead to unsoundness? I thought part of the reason why Scala's typechecking is sound is because for any type foo.bar.Baz, we need to make sure that there is an instance of foo.bar present and instantiated. But it seems that if foo.bar is a top-level object, that requirement no longer holds, since we may or may not instantiate arbitrary parts of it

@liufengyun
Copy link
Contributor

liufengyun commented Aug 28, 2024

Out of curiousity, does this behavior lead to unsoundness? I thought part of the reason why Scala's typechecking is sound is because for any type foo.bar.Baz, we need to make sure that there is an instance of foo.bar present and instantiated. But it seems that if foo.bar is a top-level object, that requirement no longer holds, since we may or may not instantiate arbitrary parts of it

It seems to me it will not impact soundness --- the static objects will be ensured to be initialized the first time they are accessed.

During the initialization of a static object, the partially initialized state might be observed, as it's the case in a normal class:

class A: // same for `object A`:
    val n = m + 1
    val m: Int = 10

class B: // same for `object B`
    class Inner:
        println(n)
    val inner = new Inner
    val n = 10

If two static objects form an initialization cycle, then uninitialized fields of the static objects might be observed (or potential deadlock in the presence of concurrency). These errors are supposed to be caught by -Ysafe-init-global for static objects, and -Wsafe-init for classes.

Edit: see #5854

@Gedochao Gedochao added area:initialization and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Sep 2, 2024
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

9 participants