Scalac dead code eliminates unused vals inside inner methods

[jvican]

Take the following expression:

@ desugar {
      def foo: String = {
        lazy val hello: String = {println("HA"); ""}
        def inner: String = {
          val unused = hello
          ""
        }
        inner
      }
}

It produces the following desugared code:

res0: Desugared = {
  def foo: scala.Predef.String = {
    lazy val hello: scala.Predef.String = {
      scala.Predef.println("HA");
      ""
    };
    def inner: scala.Predef.String = "";
    inner
  };
  ()
}

Scalac removes the call to hello when that's an illegal transformation! hello has side effects and its removal changes the semantics of a program.

I can reproduce in both 2.11.9 and 2.12.2 with Ammonite. I have also reproduced in a macro I was writing.

The error does not happen if the val definition is inside foo and not inner.

I think this is a severe bug that could be affecting the semantics of code in the wild, especially sbt in which the val _ = ??? definition is pretty common.

[jvican]

I have been playing around with this bug and I want to correct some things I've said before. This bug seems to:

• Happen when literals are present in the expr of a block.
• Be reproducible both for the inner method inner and foo.

Proofs:

• It can be reproduced inside foo.

@  desugar {
      val hello = {println("asdfasdfasfa"); ""}
      def foo: String = {
        val _ = hello
        ""
      }
    }  
res6: Desugared = {
  val hello = {
    println("asdfasdfasfa");
    ""
  };
  def foo: scala.Predef.String = "";
  ()
}

• It cannot be reproduced when either a select, ident or a complex expression is returned from a block:

@  desugar {
      val returnValue = ""
      val hello = {println("asdfasdfasfa"); ""}
      def foo: String = {
        val _ = hello
        returnValue
      }
    }  
res8: Desugared = {
  val returnValue = "";
  val hello = {
    println("asdfasdfasfa");
    ""
  };
  def foo: scala.Predef.String = {
    val _ = hello;
    returnValue
  };
  ()
}

Hence, I think the severity of this bug is not that high (we often don't return just literals). However, it can bite us in unexpected ways, especially for people not aware of this bug.

[lrytz]

The original example shows the bug, thanks. (In your second comment, I you seem to have forgotten lazy).

  def foo: String = {
    lazy val hello: String = {println("HA"); ""}
    def inner: String = {
      val unused = hello
      ""
    }

The bug seems to be in the use of isExprSafeToInline.

def typedBlock {
  ...
  treeCopy.Block(block, statsTyped, expr1)
    .setType(if (treeInfo.isExprSafeToInline(block)) expr1.tpe else expr1.tpe.deconst)
}

If the type is not deconst'd, a ConstantType is assigned to method inner, and then adapt replaces the method body by the constant's value. For some reason, a reference to a lazy type is considered "safe to inline" (that's even explicitly stated on the method's doc comment). Maybe in other contexts this is fine, but then we shouldn't use that method in typedBlock.

[jvican]

Thanks for the thorough explanation @lrytz. Indeed, I forgot the lazy in the last comment, but note that the desugared tree confirms my hypothesis :smile:.

Looking forward to seeing this fixed. Pretty impressed how fast you replied and assigned it to you!

[lrytz]

Related: in https://github.com/scala/bug/issues/7173#issuecomment-332036501, @retronym shows that removing side-effect free defs can prevent program errors from being reported.

[som-snytt]

s/unused/_/ makes it used, paradoxically.

    def inner: String = {
      val unused = hello
      "INNER"
    }

it looks like

      lazy val hello: String = "HELLO".tap(println);
      def inner: String = {
        <synthetic> <artifact> private[this] val x$1 = hello match {
          case _ => ()
        };
        "INNER"
      };

which, on the face of it, is not "more used".