scala / bug

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

false positive unused warning implicit `Unliftable` instance #12953

Closed xuwei-k closed 3 months ago

xuwei-k commented 4 months ago

Reproduction steps

Scala version: 2.13.12

build.sbt

scalaVersion := "2.13.12"

libraryDependencies += "org.scala-lang" % "scala-reflect" % scalaVersion.value

scalacOptions += "-Wunused:privates"

A.scala

package example

import scala.reflect.macros.blackbox

case class A(x: Int)

class MyMacro(val c: blackbox.Context) {
  import c.universe._

  def impl(tree: c.Tree): Tree = {
    tree match {
      case q"${a: A}" =>
        reify(()).tree
      case _ =>
        c.abort(c.enclosingPosition, "err")
    }
  }

  private implicit def instance: Unliftable[A] = ???
}

Problem

[warn] Unliftable-unused-example-project-path/A.scala:19:24: private method instance in class MyMacro is never used
[warn]   private implicit def instance: Unliftable[A] = ???
[warn]                        ^
[warn] one warning found

buf compile error if remove Unliftable[A]

[error] Unliftable-unused-example-project-path/A.scala:12:12: Can't find MyMacro.this.c.universe.Unliftable[example.A], consider providing it
[error]       case q"${a: A}" =>
[error]            ^
[error] one error found
som-snytt commented 4 months ago

This is the use case for -Wmacros:after.

But that will warn in this case for unused $m:

      def apply[U <: scala.reflect.api.Universe with Singleton]($m$untyped: scala.reflect.api.Mirror[U]): U#Tree = {
        val $u: U = $m$untyped.universe;
        val $m: $u.Mirror = $m$untyped.asInstanceOf[$u.Mirror];
        $u.Literal.apply($u.Constant.apply(()))
      }

I would expect that to be synthetic and not warnable. (reify(a) would use $m.)

You can see both warnings with -Wmacros:both.

The missing mode is "collect definitions before expansion, but collect usages both before and after". Notably, that would skip cruft in the expansion.

som-snytt commented 4 months ago

The current usage is to fiddle with -Wmacros, but the PR adds the missing mode as default so that only references (and not definitions) in the expansion contribute to reducing false positives.