surprising unused warnings due to for comprehension desugaring

[aryairani]

in 2.12.1 (and 2.11):

$ scala -Ywarn-unused
cat: /release: No such file or directory
Welcome to Scala 2.12.1 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_121).
Type in expressions for evaluation. Or try :help.

scala> object Foo {
     |   for {
     |     x <- Some(1 -> 2)
     |     y = 3
     |     (a, b) = x
     |   } yield (a + b)
     | }
<console>:14: warning: local val in value $anonfun is never used
           (a, b) = x
            ^
<console>:14: warning: local val in value $anonfun is never used
           (a, b) = x
               ^
defined object Foo

I get a warning that neither a nor b are used, although they are clearly used. No warning is issued for y, which is indeed unused.

In 2.12.2, no warnings are issued for a or b (yay) or y (boo). I acknowledge I may be doin' it wrong!

21:34 lawn-128-61-43-99:~/opensource/rho (master)$ scala -Ywarn-unused:_
Welcome to Scala 2.12.2 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_121).
Type in expressions for evaluation. Or try :help.

scala> object Foo {
     |   for {
     |     x <- Some(1 -> 2)
     |     y = 3
     |     (a, b) = x
     |   } yield (a + b)
     | }
defined object Foo

Side note, I would love to be able omit x altogether and use

object Foo {
  for {
    (a, b) <- MyObjectHavingFlatmap(1 -> 2)
  } yield (a + b)
}

without calling .filter, which is a bit scary to me.

[som-snytt]

@lrytz did a nice grid at scala/scala#5402 (comment)

It looks like there is a "fix me" in the test for this issue at scala/scala@bd28007

https://github.com/scala/scala/blob/2.12.x/test/files/neg/warn-unused-privates.scala#L188

[aryairani]

In that example you linked, I think Forever.g should warn.

[som-snytt]

For the way it works now, mid-stream assignments introduce unused cruft. But the recent approach to suppressing warnings when a refutable pattern check is emitted might be applied in more places.

There is no mechanism to track sym usage, let alone associate syms across a desugaring.

Currently, it excludes all bindings in for exprs, without which:

scala> for (ns <- List((42,17)) ; (i,j) = ns) yield 0
<console>:12: warning: pattern var ns in value $anonfun is never used; `ns@_' suppresses this warning
       for (ns <- List((42,17)) ; (i,j) = ns) yield 0
            ^
<console>:12: warning: pattern var i in value $anonfun is never used; `i@_' suppresses this warning
       for (ns <- List((42,17)) ; (i,j) = ns) yield 0
                                   ^
<console>:12: warning: pattern var j in value $anonfun is never used; `j@_' suppresses this warning
       for (ns <- List((42,17)) ; (i,j) = ns) yield 0
                                     ^
[[syntax trees at end of                     typer]] // <console>
[snip]
        private[this] val res3: List[Int] = scala.collection.immutable.List.apply[(Int, Int)](scala.Tuple2.apply[Int, Int](42, 17)).map[((Int, Int), (Int, Int)), List[((Int, Int), (Int, Int))]](((ns: (Int, Int)) => {
  <synthetic> <artifact> private[this] val x$2: ((Int, Int), Int, Int) = (ns: (Int, Int) @unchecked) match {
    case (x$1 @ (_1: Int, _2: Int)(Int, Int)((i @ _), (j @ _))) => scala.Tuple3.apply[(Int, Int), Int, Int](x$1, i, j)
  };
  val x$1: (Int, Int) = x$2._1;
  val i: Int = x$2._2;
  val j: Int = x$2._3;
  scala.Tuple2.apply[(Int, Int), (Int, Int)](ns, x$1)
}))(immutable.this.List.canBuildFrom[((Int, Int), (Int, Int))]).map[Int, List[Int]](((x$3: ((Int, Int), (Int, Int))) => (x$3: ((Int, Int), (Int, Int)) @unchecked) match {
          case (_1: (Int, Int), _2: (Int, Int))((Int, Int), (Int, Int))((ns @ _), (_1: Int, _2: Int)(Int, Int)((i @ _), (j @ _))) => 0
        }))(immutable.this.List.canBuildFrom[Int]);
        <stable> <accessor> def res3: List[Int] = $iw.this.res3
      }

Also, the comment on the test seems to be wrong, because f and g look similar after typer.

[SethTisue]

consolidating here: #11175, #11400, #12018

[martijnhoekstra]

This is not tagged as a regression, though arguably it should be (or at least in part) through #11175 which shows it regressed between 2.12.6 and 2.12.7

[SethTisue]

(Did the warning even exist before 2.12.7?)

[yashap]

@martijnhoekstra @SethTisue I see this same bug in Scala 2.11.12, so it's not new in 2.12.7, unless it was fixed in an earlier Scala 2.12 version, and then re-introduced.

[Habitats]

Any update on this? Is it realistic to hope for this getting resolved in 2.12.* ? 🙏

[som-snytt]

@Habitats the help wanted label means not really.

[kpodsiad]

@som-snytt does help wanted mean not really for 2.13.x too?

[SethTisue]

@kpodsiad it's awaiting a volunteer. nobody can forecast whether a volunteer will appear or not.

[Habitats]

So basically this isn't important enough to be prioritized?

[som-snytt]

@Habitats after -Wconf and @nowarn, there is less incentive to make a perfect warning, as the power-to-weight ratio or payoff goes down relative to the required investment. False positives are suppressible.

There is a chance something will be invented for Scala 3 that could be backported.

The current implementation could be moved to a plugin and developed independently, if a volunteer volunteered.

[Habitats]

My problem with this is that I would love to crash the compiler for unused variables, but this totally bugs out in for-comprehensions, which means that we cannot use this potentially very nice flag as part of our CI check.

Am I going at this the wrong way? 🤔

[som-snytt]

@Habitats see my previous comment, and please use one of the forums for user questions.

[som-snytt]

Welcome to Scala 2.13.10 (OpenJDK 64-Bit Server VM, Java 19).
Type in expressions for evaluation. Or try :help.

scala> for (x <- Option(42); y <- Option(27) if x < 17; res = x - y) yield res
val res0: Option[Int] = None

scala> :replay -Wunused
replay> for (x <- Option(42); y <- Option(27) if x < 17; res = x - y) yield res
                              ^
        warning: parameter value y in anonymous function is never used
val res0: Option[Int] = None

scala> :replay -Vprint:typer

      private[this] val res0: Option[Int] = scala.Option.apply[Int](42).flatMap[Int](((x: Int) => scala.Option.apply[Int](27).withFilter(((y: Int) => x.<(17))).map[(Int, Int)](((y: Int) => {
  val res: Int = x.-(y);
  scala.Tuple2.apply[Int, Int](y, res)
})).map[Int](((x$1: (Int, Int)) => (x$1: (Int, Int) @unchecked) match {
        case (_1: Int, _2: Int): (Int, Int)((y @ _), (res @ _)) => res
      }))));
      <stable> <accessor> def res0: Option[Int] = $iw.this.res0
    };

or

private[this] val res0: Option[Int] =
scala.Option.apply[Int](42)
.flatMap[Int](((x: Int) =>
  scala.Option.apply[Int](27)
  .withFilter(((y: Int) => x.<(17)))
  .map[(Int, Int)](((y: Int) => {
    val res: Int = x.-(y);
    scala.Tuple2.apply[Int, Int](y, res)
  }))
  .map[Int](((x$1: (Int, Int)) =>
    (x$1: (Int, Int) @unchecked) match {
      case (_1: Int, _2: Int): (Int, Int)((y @ _), (res @ _)) => res
    }))
));

[som-snytt]

"for-warned is fore-armed."