-Wunused reporting should wait until macro expansion for analysis

[markehammons]

Compiler version

3.3.0-RC2

Minimized code

This is hard to minimize because it's macro related. I'll add an example macro in a bit, but lets do a thought experiment. Imagine a macro whose job is to generate a method body for a method. We'll call this macro adder, and it will add together all the Int parameters of the method. It should be implementable via:

  def findMethodSymbol(using q: Quotes)(s: q.reflect.Symbol): Symbol =
    import quotes.reflect.*
    if s.isDefDef then s
    else findMethodSymbol(s.owner)
  end findMethodSymbol


  inline def adder: Int = ${
    adderImpl
  }
  def adderImpl(using Quotes): Expr[Int] =
    import quotes.reflect.*

    val inputs = findMethodSymbol(Symbol.spliceOwner).tree match
      case DefDef(_, params, _, _) =>
        params.last match
          case TermParamClause(valDefs) =>
            valDefs.map(vd => Ref(vd.symbol).asExprOf[Int])
    inputs.reduce((exp1, exp2) => '{$exp1 + $exp2})

Its usage will turn

def myMethod(a: Int, b: Int, c: Int) = adder

into

def myMethod(a: Int, b: Int, c: Int) = a + b + c

Right now, the -Wunused scanning happens too early, and it just sees adder, and the a, b, and c parameters unused. It does not see the transformed result of adder using a, b, or c.

Expectation

I would like, if possible, that the unused analysis is pushed back to after macro expansion, so that macro usage like above doesn't trigger unused explicit parameter warnings.

[mrdziuban]

I think I'm running into this as well with -Wunused:implicits and this simple code using kittens:

https://scastie.scala-lang.org/TpJADGRTQ7u6bhnud2xiug

import cats.Eq

case class Foo[A](a: List[A])
object Foo {
  given eq[A: Eq]: Eq[Foo[A]] = cats.derived.semiauto.eq[Foo[A]]
         // ^
         // unused implicit parameter
}

Removing the context bound causes a compile error on the RHS of the instance

[mrdziuban]

Do I understand correctly that the -Ywarn-macros:after option controlled this behavior in scala 2?

[Kordyjan]

We'll call this macro adder, and it will add together all the Int parameters of the method. Its usage will turn

def myMethod(a: Int, b: Int, c: Int) = adder

into

def myMethod(a: Int, b: Int, c: Int) = a + b + c

The funny thing is that this macro is really hard to write. It is not impossible, but it probably would be very fragile and cannot be moved around without risk of breaking it. Macros API seems to be created in such a way as to make the implementation of adder hard. And this macro should be considered a super-not-idiomatic hack. The idiomatic way of doing that would be either passing the parameters directly to the adder (that should accept inlined varargs) or writing it as

def myMethod = adder("a", "b", "c")

that is expanded into

def myMethod = (a: Int, b: Int, c: Int) => a + b + c

It can be even done using macro annotations, which are an experimental feature in 3.3:

@adder
def myMethod(a: Int, b: Int, c: Int) = ???

I don't think we should in any way prioritize supporting code written in such a way, and definitely, it shouldn't block the 3.3.0 release.

[Kordyjan]

@mrdziuban I'm not sure that I understand this example correctly, but the semiauto derivation seems to be existing only for the sake of compatibility with Scala 2 as it can be easily achieved in Scala 3 using derivation.

I think we can check if we can do something to eliminate false positives in cases such as yours with some risk of having false negatives.

[markehammons]

The funny thing is that this macro is really hard to write. It is not impossible, but it probably would be very fragile and cannot be moved around without risk of breaking it. Macros API seems to be created in such a way as to make the implementation of adder hard. And this macro should be considered a super-not-idiomatic hack. The idiomatic way of doing that would be either passing the parameters directly to the adder (that should accept inlined varargs) or writing it as

It's not particularly hard. It requires analysis of the half-constructed DefDef that's myMethod, but it's doable with code like such: ```scala def findMethodSymbol(using q: Quotes)(s: q.reflect.Symbol): Symbol = import quotes.reflect.* if s.isDefDef then s else findMethodSymbol(s.owner) end findMethodSymbol def adderImpl(using Quotes): Expr[Int] = import quotes.reflect.* val inputs = findMethodSymbol(Symbol.spliceOwner).tree match case DefDef(_, params, _, _) => params.last match case TermParamClause(valDefs) => valDefs.map(vd => Ref(vd.symbol).asExprOf[Int]) inputs.reduce((exp1, exp2) => '{$exp1 + $exp2}) ``` Can this implementation be error prone? Yes, multi-clause methods won't work properly, nor methods that have non-int params. Guards against such things can easily be added though. The problem with the lambda example is that it's not as performant as code that defines the body of a method. Macro annotations probably have an easier time finding the symbol they're working on, and would be a welcome addition, but you'd still have this warning in the case of a macro annotation that updates the body of myMethod with `a + b + c`.

[Kordyjan]

Still, I think

def myMethod(a: Int, b: Int, c: Int) = adder(a, b, c)

is the best way to go here, from the perspective of readability, performance, and maintainability, and should be considered the best way of doing this.

Of course, I think we can revisit why the checking of unused symbols was put where it was. Your problem will be fixed one way or the other in the future. However, I still believe that use cases affected by this problem are so rare that we don't need to delay the 3.3 because of that, and it can be fixed later (this is why I removed the issue from the milestone, and not closed it).

[markehammons]

Your version is not wrong, but may be undesirable in some cases. adder here is just a reduced sample of something I'm doing with real code:

  def abs(i: Int): Int = binding

expanding into something like

def abs(i: Int): Int =  TransitionModule.methodReturn(IntDescriptor, absHandle.invokeExact(TransitionModule.methodArgument(IntDescriptor, i): Int))

Having the user needing to manually specify inputs to the macro doesn't really help because in this case, the real input to the macro is the method definition it's attached to.

Yes, macro annotations will likely end up being a better tool for this, but they aren't here yet, and for the work I'm doing, I'm frequently having to find non-experimental alternatives to things that would be better done with features that are still marked experimental in Scala 3.

I understand that this isn't high priority because the "proper" support in Scala 3 for this pattern is still marked experimental, but it's still kind of disappointing that this pattern will produce false-positives for unused params till perhaps much later. Especially since Scala 3.3 is supposed to be an LTS version and I'd like to have a version of my library working nicely for it.