scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

Let `markForAsyncTransform` work with scalac-option `-Wnonunit-statement` #12723

Closed cornerman closed 1 year ago

cornerman commented 1 year ago

I am using scala-async with the -xasync compiler option. See the issue there as well: https://github.com/scala/scala-async/issues/316. Using this library (which uses markForAsyncTransform under the hoods) gives false warnings with the rather new scalac-option -Wnonunit-statement.

Reproduction steps

Scala version: 2.13.9

This is an example with scala-async that will trigger the warning (I do not know how to reproduce without a library):

  def test(): Future[Boolean] =
    async {
      await(Future.successful(true))
    }

This will yield a warning:

unused value of type Boolean (add `: Unit` to discard silently)
[warn]     async {

The generated/desugared code from the scala-async macro involving markForAsyncTransform looks like this:

{
  final class stateMachine$async extends _root_.scala.async.FutureStateMachine {
    def <init>(): stateMachine$async = {
      stateMachine$async.super.<init>(scala.concurrent.ExecutionContext.Implicits.global);
      ()
    };
    override def apply(tr$async: scala.util.Try[AnyRef]): Unit = {
      scala.Predef.locally[Boolean](scala.async.Async.await[Boolean](scala.concurrent.Future.successful[Boolean](true)));
      ()
    }
  };
  (new stateMachine$async().start[Nothing](): scala.concurrent.Future[Boolean])
}

The body of the above apply method ignores the Boolean value that is returned in the first line.

Problem

The code generated by the markForAsyncTransform method (see https://github.com/scala/scala/blob/2bd33021228e0dc916f34480bdbe07bbcee3f41d/src/compiler/scala/tools/nsc/transform/async/AsyncPhase.scala#L42) will generate a call to scala.PreDef.locally which actually returns a value, but that value is ignored. Therefore the scalac-option -Wnonunit-statement will render a warning. If we would add a : Unit ascription to that statement, it should pass without warnings.

SethTisue commented 1 year ago

Sounds very plausible — interested in trying to PR it...?

cornerman commented 1 year ago

@SethTisue Yes, I am interested in doing that :)

I did not have the time yet, but I will try to start it soon!

lrytz commented 1 year ago

https://github.com/scala/scala/pull/10419