Merge pull request #10693 from som-snytt/issue/12953-unliftable-unapply

For macros look for usages in expansion by default

src/compiler/scala/tools/nsc/Global.scala

@@ -1271,7 +1271,7 @@ class Global(var currentSettings: Settings, reporter0: Reporter)
           } else
             skippable || !pd.enabled
         }
-        val phs = phaseDescriptors takeWhile unstoppable filterNot skippable
+        val phs = phaseDescriptors.takeWhile(unstoppable).filterNot(skippable)
         // Ensure there is a terminal phase at the end, since -Ystop may have limited the phases.
         if (phs.isEmpty || !phs.last.terminal) {
           val t = if (phaseDescriptors.last.terminal) phaseDescriptors.last else terminal

src/compiler/scala/tools/nsc/settings/Warnings.scala

@@ -103,13 +103,14 @@ trait Warnings {
     name    = "-Wmacros",
     helpArg = "mode",
     descr   = "Enable lint warnings on macro expansions.",
-    choices = List("none", "before", "after", "both"),
-    default = "before",
+    choices = List("none", "before", "after", "both", "default"),
+    default = "default",
     choicesHelp = List(
       "Do not inspect expansions or their original trees when generating unused symbol warnings.",
       "Only inspect unexpanded user-written code for unused symbols.",
       "Only inspect expanded trees when generating unused symbol warnings.",
-      "Inspect both user-written code and expanded trees when generating unused symbol warnings."
+      "Inspect both user-written code and expanded trees when generating unused symbol warnings.",
+      "Only inspect unexpanded user-written code for unused symbols but include usages in expansions.",
     )
   ) withAbbreviation "-Ywarn-macros"
   val warnDeadCode         = BooleanSetting("-Wdead-code", "Warn when dead code is identified.") withAbbreviation "-Ywarn-dead-code"

src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala

@@ -505,12 +505,12 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
   class UnusedPrivates extends Traverser {
     import UnusedPrivates.ignoreNames
     def isEffectivelyPrivate(sym: Symbol): Boolean = false
-    val defnTrees = ListBuffer[MemberDef]()
-    val targets   = mutable.Set[Symbol]()
-    val setVars   = mutable.Set[Symbol]()
-    val treeTypes = mutable.Set[Type]()
-    val params    = mutable.Set[Symbol]()
-    val patvars   = mutable.Set[Symbol]()
+    val defnTrees = ListBuffer.empty[MemberDef]
+    val targets   = mutable.Set.empty[Symbol]
+    val setVars   = mutable.Set.empty[Symbol]
+    val treeTypes = mutable.Set.empty[Type]
+    val params    = mutable.Set.empty[Symbol]
+    val patvars   = mutable.Set.empty[Symbol]
 
     def varsWithoutSetters = defnTrees.iterator.map(_.symbol).filter(t => t.isVar && !isExisting(t.setter))
 
@@ -669,19 +669,45 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
 
   class checkUnused(typer: Typer) {
 
+    private def isMacroAnnotationExpansion(tree: Tree): Boolean = tree.hasSymbolField && isExpanded(tree.symbol)
+
+    private def isMacroExpansion(tree: Tree): Boolean = hasMacroExpansionAttachment(tree) || isMacroAnnotationExpansion(tree)
+
     object skipMacroCall extends UnusedPrivates {
       override def qualifiesTerm(sym: Symbol): Boolean =
         super.qualifiesTerm(sym) && !sym.isMacro
     }
     object skipMacroExpansion extends UnusedPrivates {
-      override def traverse(t: Tree): Unit =
-        if (!hasMacroExpansionAttachment(t) && !(t.hasSymbolField && isExpanded(t.symbol)))
-          super.traverse(t)
+      override def traverse(tree: Tree): Unit = if (!isMacroExpansion(tree)) super.traverse(tree)
     }
     object checkMacroExpandee extends UnusedPrivates {
-      override def traverse(t: Tree): Unit =
-        if (!(t.hasSymbolField && isExpanded(t.symbol)))
-          super.traverse(if (hasMacroExpansionAttachment(t)) macroExpandee(t) else t)
+      override def traverse(tree: Tree): Unit =
+        if (!isMacroAnnotationExpansion(tree))
+          super.traverse(if (hasMacroExpansionAttachment(tree)) macroExpandee(tree) else tree)
+    }
+    // collect definitions and refs from expandee (and normal trees) but only refs from expanded trees
+    object checkMacroExpandeeAndExpandedRefs extends UnusedPrivates {
+      object refCollector extends Traverser {
+        override def traverse(tree: Tree): Unit = {
+          tree match {
+            case _: RefTree if isExisting(tree.symbol) => targets += tree.symbol
+            case _ =>
+          }
+          if (tree.tpe != null) tree.tpe.prefix.foreach {
+            case SingleType(_, sym) => targets += sym
+            case _ =>
+          }
+          super.traverse(tree)
+        }
+      }
+      override def traverse(tree: Tree): Unit =
+        if (hasMacroExpansionAttachment(tree)) {
+          super.traverse(macroExpandee(tree))
+          refCollector.traverse(tree)
+        }
+        else if (isMacroAnnotationExpansion(tree))
+          refCollector.traverse(tree)
+        else super.traverse(tree)
     }
 
     private def warningsEnabled: Boolean = {
@@ -793,6 +819,7 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
       val body = unit.body
       // TODO the message should distinguish whether the non-usage is before or after macro expansion.
       settings.warnMacros.value match {
+        case "default"=> run(checkMacroExpandeeAndExpandedRefs)(body)
         case "none"   => run(skipMacroExpansion)(body)
         case "before" => run(checkMacroExpandee)(body)
         case "after"  => run(skipMacroCall)(body)

src/compiler/scala/tools/nsc/typechecker/Typers.scala

@@ -6017,22 +6017,26 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
         // attempt to avoid warning about trees munged by macros
         def isMacroExpansion = {
           // context.tree is not the expandee; it is plain new SC(ps).m(args)
-          //context.tree exists (t => (t.pos includes lit.pos) && hasMacroExpansionAttachment(t))
+          //context.tree.exists(t => t.pos.includes(lit.pos) && hasMacroExpansionAttachment(t))
           // testing pos works and may suffice
-          //openMacros exists (_.macroApplication.pos includes lit.pos)
+          //openMacros.exists(_.macroApplication.pos.includes(lit.pos))
           // tests whether the lit belongs to the expandee of an open macro
-          openMacros exists (_.macroApplication.attachments.get[MacroExpansionAttachment] match {
-            case Some(MacroExpansionAttachment(_, t: Tree)) => t exists (_ == lit)
+          openMacros.exists(_.macroApplication.attachments.get[MacroExpansionAttachment] match {
+            case Some(MacroExpansionAttachment(_, t: Tree)) => t.exists(_ eq lit)
             case _                                          => false
           })
         }
+        val checkMacroExpansion = settings.warnMacros.value match {
+          case "both" | "after" => true
+          case _ => !isMacroExpansion
+        }
         // An interpolation desugars to `StringContext(parts).m(args)`, so obviously not missing.
         // `implicitNotFound` annotations have strings with `${A}`, so never warn for that.
-        // Also don't warn for macro expansion.
+        // Also don't warn for macro expansion unless they ask for it.
         def mightBeMissingInterpolation: Boolean = context.enclosingApply.tree match {
           case Apply(Select(Apply(RefTree(_, nme.StringContextName), _), _), _) => false
           case Apply(Select(New(RefTree(_, tpnme.implicitNotFound)), _), _)     => false
-          case _                                                                => !isMacroExpansion
+          case _                                                                => checkMacroExpansion
         }
         def maybeWarn(s: String): Unit = {
           def warn(message: String) = context.warning(lit.pos, s"possible missing interpolator: $message", WarningCategory.LintMissingInterpolator)
@@ -6074,7 +6078,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
       def typedLiteral(tree: Literal) = {
         if (settings.warnMissingInterpolator) warnMissingInterpolator(tree)
 
-        tree setType (if (tree.value.tag == UnitTag) UnitTpe else ConstantType(tree.value))
+        tree.setType(if (tree.value.tag == UnitTag) UnitTpe else ConstantType(tree.value))
       }
 
       def typedSingletonTypeTree(tree: SingletonTypeTree) = {

test/files/neg/t10296-both/Unused_2.scala

@@ -1,4 +1,4 @@
-// scalac: -Werror -Xlint:unused -Wmacros:both
+//> using options -Werror -Xlint:unused -Wmacros:both
 import scala.language.experimental.macros
 
 object Unused extends App {

test/files/neg/t12953-expandee.check

@@ -0,0 +1,9 @@
+Client_2.scala:10: warning: possible missing interpolator: detected interpolated identifier `$unusedVariable`
+      println("hello, world of $unusedVariable")
+              ^
+Client_2.scala:9: warning: local val unusedVariable in value <local Test> is never used
+      val unusedVariable = "42".toInt
+          ^
+error: No warnings can be incurred under -Werror.
+2 warnings
+1 error

test/files/neg/t12953-expandee/Client_2.scala

@@ -0,0 +1,13 @@
+
+//> using options -Werror -Wunused:locals -Xlint:missing-interpolator -Wmacros:before
+
+import Macro.id
+
+object Test extends App {
+  println {
+    id {
+      val unusedVariable = "42".toInt
+      println("hello, world of $unusedVariable")
+    }
+  }
+}

test/files/neg/t12953-expandee/Macro_1.scala

@@ -0,0 +1,11 @@
+
+import scala.language.experimental.macros
+import scala.reflect.macros.blackbox.Context
+
+object Macro {
+  def id[A](body: A): A = macro impl[A]
+
+  def impl[A: c.WeakTypeTag](c: Context)(body: c.Expr[A]) = {
+    body
+  }
+}

test/files/neg/t12953-expansion-b.check

@@ -0,0 +1,9 @@
+Client_2.scala:8: warning: possible missing interpolator: detected interpolated identifier `$unusedVariable`
+    id {
+       ^
+Client_2.scala:8: warning: local val unusedVariable in value <local Test> is never used
+    id {
+       ^
+error: No warnings can be incurred under -Werror.
+2 warnings
+1 error

test/files/neg/t12953-expansion-b/Client_2.scala

@@ -0,0 +1,12 @@
+
+//> using options -Werror -Wunused:locals -Xlint:missing-interpolator -Wmacros:after
+
+import Macro.id
+
+object Test extends App {
+  println {
+    id {
+      println("goodbye, cruel world of $unusedVariable")
+    }
+  }
+}

test/files/neg/t12953-expansion-b/Macro_1.scala

@@ -0,0 +1,13 @@
+
+import scala.language.experimental.macros
+import scala.reflect.macros.blackbox.Context
+
+// with unused interpolator check in typer, the variable and literal must be typechecked together to warn
+object Macro {
+  def id[A](body: A): A = macro impl[A]
+
+  def impl[A: c.WeakTypeTag](c: Context)(body: c.Expr[A]) = {
+    import c.universe._
+    q"""val unusedVariable = "42".toInt; println("hello, world of $$unusedVariable"); $body"""
+  }
+}

test/files/neg/t12953-expansion.check

@@ -0,0 +1,6 @@
+Client_2.scala:8: warning: local val unusedVariable in value <local Test> is never used
+    id {
+       ^
+error: No warnings can be incurred under -Werror.
+1 warning
+1 error

test/files/neg/t12953-expansion/Client_2.scala

@@ -0,0 +1,12 @@
+
+//> using options -Werror -Wunused:locals -Xlint:missing-interpolator -Wmacros:after
+
+import Macro.id
+
+object Test extends App {
+  println {
+    id {
+      println("hello, world of $unusedVariable")
+    }
+  }
+}

test/files/neg/t12953-expansion/Macro_1.scala

@@ -0,0 +1,13 @@
+
+import scala.language.experimental.macros
+import scala.reflect.macros.blackbox.Context
+
+// with unused interpolator check in typer, the variable and literal must be typechecked together to warn
+object Macro {
+  def id[A](body: A): A = macro impl[A]
+
+  def impl[A: c.WeakTypeTag](c: Context)(body: c.Expr[A]) = {
+    import c.universe._
+    q"""val unusedVariable = "42".toInt; $body"""
+  }
+}

test/files/pos/macro-annot-unused-param-b/Macros_1.scala

@@ -0,0 +1,22 @@
+//> using options -Ymacro-annotations
+import scala.language.experimental.macros
+import scala.reflect.macros.blackbox.Context
+import scala.annotation.StaticAnnotation
+
+object Macros {
+  def annotImpl(c: Context)(annottees: c.Expr[Any]*): c.Expr[Any] = {
+    import c.universe._
+    val classTree = annottees.head.tree
+    val objectTree = q"""
+    object X {
+      def f: Int => String = { x => "hello" }
+    }
+    """
+
+    c.Expr[Any](Block(List(classTree, objectTree), Literal(Constant(()))))
+  }
+}
+
+class mymacro extends StaticAnnotation {
+  def macroTransform(annottees: Any*): Any = macro Macros.annotImpl
+}

test/files/pos/macro-annot-unused-param-b/Test_2.scala

@@ -0,0 +1,7 @@
+//> using options -Ymacro-annotations -Werror -Wmacros:before -Wunused:params
+@mymacro
+class X
+
+object Test {
+  println(X.f(123))
+}

test/files/pos/macro-annot-unused-param/Macros_1.scala

@@ -1,4 +1,4 @@
-// scalac: -Ymacro-annotations
+//> using options -Ymacro-annotations
 import scala.language.experimental.macros
 import scala.reflect.macros.blackbox.Context
 import scala.annotation.StaticAnnotation

test/files/pos/macro-annot-unused-param/Test_2.scala

@@ -1,4 +1,4 @@
-// scalac: -Ymacro-annotations -Wunused:params -Werror
+//> using options -Ymacro-annotations -Werror -Wmacros:default -Wunused:params
 @mymacro
 class X
 

test/files/pos/t12953-expandee/Client_2.scala

@@ -0,0 +1,12 @@
+
+//> using options -Werror -Wunused:locals -Xlint:missing-interpolator -Wmacros:default
+
+import Macro.id
+
+object Test extends App {
+  println {
+    id {
+      println("hello, world of $unusedVariable")
+    }
+  }
+}

test/files/pos/t12953-expandee/Macro_1.scala

@@ -0,0 +1,12 @@
+
+import scala.language.experimental.macros
+import scala.reflect.macros.blackbox.Context
+
+object Macro {
+  def id[A](body: A): A = macro impl[A]
+
+  def impl[A: c.WeakTypeTag](c: Context)(body: c.Expr[A]) = {
+    import c.universe._
+    q"""val unusedVariable = "42".toInt; $body"""
+  }
+}

test/files/pos/t12953.scala

@@ -0,0 +1,24 @@
+
+//> using options -Werror -Wunused:privates
+
+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(a).tree // uses $m in expansion
+        reify(()).tree
+      case _ =>
+        c.abort(c.enclosingPosition, "err")
+    }
+  }
+
+  private implicit def instance: Unliftable[A] = ???
+}