throw inside async block causes dead code warning

[jrray]

scala> async { throw new Exception }
<console>:13: warning: dead code following this construct

This block does return a failed Future as expected, but the warning is a cosmetic problem. Is it possible to remove? Or is there a better way to fail an async block?

[retronym]

This warning is issued before the macro is expanded. To avoid this, we'd probably need to change the type signature of await to use a by-name parameter.

I'd recommend using Future.failed(new Exception) for this use case.

[dwhjames]

I’ve also encountered dead code warnings with the -Ywarn-dead-code compiler options.

Please excuse the strangeness of the example that follows, this has been whittled down from a much larger code snippet.

$ sbt -Dscala.async.debug=true console

scala> :paste
// Entering paste mode (ctrl-D to finish)

import scala.async.Async._
import scala.concurrent.duration.Duration
import scala.concurrent.{Await, Future}
import scala.concurrent.ExecutionContext.Implicits.global

def foo() = async {
  val opt = await(async(Option.empty[String => Future[Unit]]))
  opt match {
    case None =>
      throw new RuntimeException("case a")
    case Some(f) =>
      await(f("case b"))
  }
}

// Exiting paste mode, now interpreting.

The warning is that there is dead code after the throw.

The relevant snippet of the state machine is copied below:

[async] async state machine transform expands to:
…
            case 3 => {
              {
                ();
                stateMachine$macro$29.this.f$bind$macro$34$macro$37 = null
              };
              {
                ();
                stateMachine$macro$29.this.await$macro$31$macro$35 = null
              };
              throw new scala.`package`.RuntimeException("case a");
              stateMachine$macro$29.this.state = 5;
              ()
            }
…
            case 5 => {
              {
                stateMachine$macro$29.this.result.complete(Success.apply[Unit](()));
                ()
              };
              return ()
            }
…

The dead code is pretty obvious now.

I can eliminate the dead code warning if I refactor to

scala> :paste
// Entering paste mode (ctrl-D to finish)
def foo() = async {
  val opt = await(async(Option.empty[String => Future[Unit]]))
  await { opt match {
    case None =>
      throw new RuntimeException("case a")
    case Some(f) =>
      f("case b")
  }}
}
// Exiting paste mode, now interpreting.

The relevant case in the state machine is now:

[async] async state machine transform expands to:
…
            case 2 => {
              val opt: Option[String => scala.concurrent.Future[Unit]] = stateMachine$macro$40.this.await$macro$42$macro$47;
              <synthetic> var matchres$macro$43: scala.concurrent.Future[Unit] @scala.reflect.internal.annotations.uncheckedBounds = null;
              opt match {
                case scala.None => matchres$macro$43 = (throw new scala.`package`.RuntimeException("case a").asInstanceOf[scala.concurrent.Future[Unit] @scala.reflect.internal.annotations.uncheckedBounds]: scala.concurrent.Future[Unit] @scala.reflect.internal.annotations.uncheckedBounds)
                case (x: String => scala.concurrent.Future[Unit])Some[String => scala.concurrent.Future[Unit]]((f @ _)) => matchres$macro$43 = (f.apply("case b").asInstanceOf[scala.concurrent.Future[Unit] @scala.reflect.internal.annotations.uncheckedBounds]: scala.concurrent.Future[Unit] @scala.reflect.internal.annotations.uncheckedBounds)
              };
              <synthetic> val awaitable$macro$44: scala.concurrent.Future[Unit] @scala.reflect.internal.annotations.uncheckedBounds = matchres$macro$43;
              stateMachine$macro$40.this.state = 3;
              if (awaitable$macro$44.isCompleted)
                {
                  if (awaitable$macro$44.value.get.isFailure)
                    {
                      {
                        stateMachine$macro$40.this.result.complete(awaitable$macro$44.value.get.asInstanceOf[scala.util.Try[Unit]]);
                        ()
                      };
                      return ()
                    }
                  else
                    {
                      stateMachine$macro$40.this.await$macro$45$macro$46 = awaitable$macro$44.value.get.get.asInstanceOf[Unit];
                      stateMachine$macro$40.this.state = 4
                    };
                  ()
                }
              else
                {
                  awaitable$macro$44.onComplete[Unit](this)(stateMachine$macro$40.this.execContext);
                  return ()
                };
              ()
            }
…

Out of interest I changed to the throw … to a Future.failed(…)

scala> :paste
// Entering paste mode (ctrl-D to finish)

def foo() = async {
  val opt = await(async(Option.empty[String => Future[Unit]]))
  await { opt match {
    case None =>
      Future.failed(new RuntimeException("case a"))
    case Some(f) =>
      f("case b")
  }}
}

// Exiting paste mode, now interpreting.

And the state machine is much the same:

…
            case 2 => {
              val opt: Option[String => scala.concurrent.Future[Unit]] = stateMachine$macro$50.this.await$macro$52$macro$56;
              <synthetic> var matchres$macro$53: scala.concurrent.Future[Unit] @scala.reflect.internal.annotations.uncheckedBounds = null;
              opt match {
                case scala.None => matchres$macro$53 = (scala.concurrent.Future.failed[Nothing](new scala.`package`.RuntimeException("case a")).asInstanceOf[scala.concurrent.Future[Unit] @scala.reflect.internal.annotations.uncheckedBounds]: scala.concurrent.Future[Unit] @scala.reflect.internal.annotations.uncheckedBounds)
                case (x: String => scala.concurrent.Future[Unit])Some[String => scala.concurrent.Future[Unit]]((f @ _)) => matchres$macro$53 = (f.apply("case b").asInstanceOf[scala.concurrent.Future[Unit] @scala.reflect.internal.annotations.uncheckedBounds]: scala.concurrent.Future[Unit] @scala.reflect.internal.annotations.uncheckedBounds)
              };
              <synthetic> val awaitable$macro$54: scala.concurrent.Future[Unit] @scala.reflect.internal.annotations.uncheckedBounds = matchres$macro$53;
              stateMachine$macro$50.this.state = 3;
              if (awaitable$macro$54.isCompleted)
                {
                  if (awaitable$macro$54.value.get.isFailure)
                    {
                      {
                        stateMachine$macro$50.this.result.complete(awaitable$macro$54.value.get.asInstanceOf[scala.util.Try[Unit]]);
                        ()
                      };
                      return ()
                    }
                  else
                    {
                      stateMachine$macro$50.this.await$macro$55$macro$57 = awaitable$macro$54.value.get.get.asInstanceOf[Unit];
                      stateMachine$macro$50.this.state = 4
                    };
                  ()
                }
              else
                {
                  awaitable$macro$54.onComplete[Unit](this)(stateMachine$macro$50.this.execContext);
                  return ()
                };
              ()
            }
…

Despite the dead code in the original transformation… it doesn’t seem like anything is necessarily going wrong here… right?

[retronym]

@dwhjames I can't reproduce the warning, although I can see the dead code. Could you please post a transcript that shows the warning in context?

Maybe the warning only shows up after untypechecking / retypechecking the expansion of the async macro?

[dwhjames]

I dropped my first snippet in an object in the project’s test scope, and got:

[warn] scala-async/src/test/scala/scala/async/foo.scala:11: dead code following this construct
[warn]         throw new RuntimeException("case a")
[warn]         ^

after adding the warning option in test scope:

scalacOptions in Test ++= Seq("-Yrangepos", "-Ywarn-dead-code")

In fact, this shows a lot of other dead code warnings too, in files such as AnfTransformSpec.scala

[retronym]

Thanks, I can reproduce. Seems like range positions are required, oddly enough.

I think the easiest fix is to wrap any Nothing typed expressions in <expr>: Any when we construct the state machine.

The other approach I'm trying is to emit try { user code } finally { generated code } rather than { user code; generated code }. This might be important if when we get around to supporting try / catch in the async transform. Right now, we don't really care that we haven't advanced the state after an exception is thrown, as it always aborts the entire async block.

[retronym]

WIP branch to fix both issues here: https://github.com/scala/async/compare/master...retronym:ticket/105?expand=1

[retronym]

@dwhjames Would you mind verifying this against the master branch? I've merged my fix.

[dwhjames]

@retronym I’ve tested the head of master and everything seems to compile and run fine with the project I’m working on.

However, I went back to 0.9.5 and 0.9.3 and couldn’t trigger any dead code warnings. Turns out my work around for #120 of including an explicit return type is related.

• on 0.9.3, without the return type, I had the NotImplementedError issue reported in NotImplementedError is being thrown in... #120, and also a dead code warning
• on 0.9.5, without the return type, your fix for NotImplementedError is being thrown in... #120 solves the NotImplementedError, but there was still a dead code warning

In any case, your fix in #132 doesn’t appear to have any regressions for my project. (I didn’t have any instances of the original example filed in this ticket.)

However, setting

scalacOptions in Test ++= Seq("-Yrangepos", "-Ywarn-dead-code")

for the scala-async build still shows lots of dead code warnings. Is this still to be expected?

Also, you commented about -Yrangepos. Is this something that one should typically have turned on (I couldn’t find much info about exactly what it does). And I noted that this is only turned on here for tests, which seems to stem from c780810 referencing SI-7649

[retronym]

Thanks for the feedback. I'm working on a few more instances of the problem that this has uncovered.