Inliner can remove side effects from objects

[Blaisorblade]

Noticed while discussing #4763, haven't found in the tracker. Might be a known bug (related to what @allanrenucci noticed recently with constant folding accepting idempotent rather than pure methods), but we should have an issue for this.

EDIT: the issue is that println("object init"); is dropped.

sbt:dotty> repl -Xprint:frontend
object A {
  println("object init");
  inline def x: 1 = { println("x init") ; 1 } //`inline val` requires a val on the rhs
}
scala> object A {
           println("object init");
           inline def x: 1 = { println("x init") ; 1 } //`inline val` requires a val on the rhs
         }

result of rs$line$1 after frontend:
package <empty> {
  final lazy module val rs$line$1: rs$line$1$ = new rs$line$1$()
  final module class rs$line$1$() extends Object() { this: rs$line$1.type =>
    final lazy module val A: A$ = new A$()
    final module class A$() extends Object() { this: A.type =>
      println("object init")
      inline def x: 1.type =
        {
          println("x init")
          1
        }
    }
  }
}
// defined object A

scala>

scala> object B {
           val y = A.x
         }
result of rs$line$2 after frontend:
package <empty> {
  final lazy module val rs$line$2: rs$line$2$ = new rs$line$2$()
  final module class rs$line$2$() extends Object() { this: rs$line$2.type =>
    final lazy module val B: B$ = new B$()
    final module class B$() extends Object() { this: B.type =>
      val y: Int =
        /* inlined from A.x*/
          {
            {
              println("x init")
              1
            }
          }
    }
  }
}
// defined object B

[Blaisorblade]

The known issue is https://github.com/lampepfl/dotty/blob/c3469da3538546cd8d7311da932f68b76737b909/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala#L416-L459, but I don't think that's the code involved here.

[lrytz]

See also scala/scala-dev#112

[Blaisorblade]

Uh, the prefix will be invoked, but not if it's idempotent — the comment disagrees with the code and it seems the comment's right.
https://github.com/lampepfl/dotty/blob/31f9a1b79f905d128eb6bf42d5900f62de050c49/compiler/src/dotty/tools/dotc/typer/Inliner.scala#L422-L423

@milessabin asks in #4763:

I'm still unclear how this is supposed to interact with object initialization. Is the intention to check object bodies for purity and ban inline definitions if the object body contains any effects?

I hadn't realized, but that's an option in https://github.com/lampepfl/dotty/blob/c3469da3538546cd8d7311da932f68b76737b909/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala#L416-L459, which says

Then we can demand purity of the prefix unless the selection goes to an inline val.

When we talked about this code this Monday in the meeting, we didn't change this code because our purity checker is still too weak.
Alternatively, I think we should just actually reference A. Producing {A; { println("x init") ; 1 } } + 1 is correct whether or not A has effectful initialization code, thanks to the lazy semantics of references to objects.

Furthermore, there's a micro-purity checking (being extended), and references to an object are marked as idempotent; that is, we know it's safe to replace A; A by A. But we can only drop that reference to A if it's pure.

[odersky]

I think we should leave this as is for the moment. The problem is that, without a good purity analysis of objects we end up referring to way too many objects. The inliner's job is to produce fast code, and it can't do that if we fix this issue.

I believe the correct approach is to come up with a better purity analysis of objects. #4710 is a start. If we have that, we can switch to isImpure but not before.

[sjrd]

The inliner's job is to produce fast code, and it can't do that if we fix this issue.

I'm sorry but no. The inliner's primary job is to produce correct code. Under that constraint, it should make it as fast as possible. But disregarding correctness because it's annoying to produce fast code is a terrible mistake.

If you want to do that, you need to change the language specification to relax the rules of object initialization.