Make CheckUnused run both after Typer and Inlining (#17206)

Resolve #16876

compiler/src/dotty/tools/dotc/Compiler.scala

@@ -35,7 +35,7 @@ class Compiler {
   protected def frontendPhases: List[List[Phase]] =
     List(new Parser) ::             // Compiler frontend: scanner, parser
     List(new TyperPhase) ::         // Compiler frontend: namer, typer
-    List(new CheckUnused) ::        // Check for unused elements
+    List(new CheckUnused.PostTyper) ::  // Check for unused elements
     List(new YCheckPositions) ::    // YCheck positions
     List(new sbt.ExtractDependencies) :: // Sends information on classes' dependencies to sbt via callbacks
     List(new semanticdb.ExtractSemanticDB) :: // Extract info into .semanticdb files
@@ -50,6 +50,7 @@ class Compiler {
     List(new Pickler) ::            // Generate TASTY info
     List(new Inlining) ::           // Inline and execute macros
     List(new PostInlining) ::       // Add mirror support for inlined code
+    List(new CheckUnused.PostInlining) ::  // Check for unused elements
     List(new Staging) ::            // Check staging levels and heal staged types
     List(new Splicing) ::           // Replace level 1 splices with holes
     List(new PickleQuotes) ::       // Turn quoted trees into explicit run-time data structures

compiler/src/dotty/tools/dotc/transform/CheckUnused.scala

@@ -25,30 +25,23 @@ import dotty.tools.dotc.core.Definitions
 import dotty.tools.dotc.core.NameKinds.WildcardParamName
 import dotty.tools.dotc.core.Symbols.Symbol
 import dotty.tools.dotc.core.StdNames.nme
-
+import scala.math.Ordering
 
 /**
  * A compiler phase that checks for unused imports or definitions
  *
  * Basically, it gathers definition/imports and their usage. If a
  * definition/imports does not have any usage, then it is reported.
  */
-class CheckUnused extends MiniPhase:
-  import CheckUnused.UnusedData
-
-  /**
-   * The key used to retrieve the "unused entity" analysis metadata,
-   * from the compilation `Context`
-   */
-  private val _key = Property.Key[UnusedData]
+class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _key: Property.Key[CheckUnused.UnusedData]) extends MiniPhase:
+  import CheckUnused.*
+  import UnusedData.*
 
   private def unusedDataApply[U](f: UnusedData => U)(using Context): Context =
     ctx.property(_key).foreach(f)
     ctx
-  private def getUnusedData(using Context): Option[UnusedData] =
-    ctx.property(_key)
 
-  override def phaseName: String = CheckUnused.phaseName
+  override def phaseName: String = CheckUnused.phaseNamePrefix + suffix
 
   override def description: String = CheckUnused.description
 
@@ -60,13 +53,21 @@ class CheckUnused extends MiniPhase:
 
   override def prepareForUnit(tree: tpd.Tree)(using Context): Context =
     val data = UnusedData()
+    tree.getAttachment(_key).foreach(oldData =>
+      data.unusedAggregate = oldData.unusedAggregate
+    )
     val fresh = ctx.fresh.setProperty(_key, data)
+    tree.putAttachment(_key, data)
     fresh
 
   // ========== END + REPORTING ==========
 
   override def transformUnit(tree: tpd.Tree)(using Context): tpd.Tree =
-    unusedDataApply(ud => reportUnused(ud.getUnused))
+    unusedDataApply { ud =>
+      ud.finishAggregation()
+      if(phaseMode == PhaseMode.Report) then
+        ud.unusedAggregate.foreach(reportUnused)
+    }
     tree
 
   // ========== MiniPhase Prepare ==========
@@ -252,31 +253,35 @@ class CheckUnused extends MiniPhase:
   private def traverseAnnotations(sym: Symbol)(using Context): Unit =
     sym.denot.annotations.foreach(annot => traverser.traverse(annot.tree))
 
+
   /** Do the actual reporting given the result of the anaylsis */
   private def reportUnused(res: UnusedData.UnusedResult)(using Context): Unit =
-    import CheckUnused.WarnTypes
-    res.warnings.foreach { s =>
+    res.warnings.toList.sortBy(_.pos.line)(using Ordering[Int]).foreach { s =>
       s match
-        case (t, WarnTypes.Imports) =>
+        case UnusedSymbol(t, _, WarnTypes.Imports) =>
           report.warning(s"unused import", t)
-        case (t, WarnTypes.LocalDefs) =>
+        case UnusedSymbol(t, _, WarnTypes.LocalDefs) =>
           report.warning(s"unused local definition", t)
-        case (t, WarnTypes.ExplicitParams) =>
+        case UnusedSymbol(t, _, WarnTypes.ExplicitParams) =>
           report.warning(s"unused explicit parameter", t)
-        case (t, WarnTypes.ImplicitParams) =>
+        case UnusedSymbol(t, _, WarnTypes.ImplicitParams) =>
           report.warning(s"unused implicit parameter", t)
-        case (t, WarnTypes.PrivateMembers) =>
+        case UnusedSymbol(t, _, WarnTypes.PrivateMembers) =>
           report.warning(s"unused private member", t)
-        case (t, WarnTypes.PatVars) =>
+        case UnusedSymbol(t, _, WarnTypes.PatVars) =>
           report.warning(s"unused pattern variable", t)
     }
 
 end CheckUnused
 
 object CheckUnused:
-  val phaseName: String = "checkUnused"
+  val phaseNamePrefix: String = "checkUnused"
   val description: String = "check for unused elements"
 
+  enum PhaseMode:
+    case Aggregate
+    case Report
+
   private enum WarnTypes:
     case Imports
     case LocalDefs
@@ -285,19 +290,30 @@ object CheckUnused:
     case PrivateMembers
     case PatVars
 
+  /**
+   * The key used to retrieve the "unused entity" analysis metadata,
+   * from the compilation `Context`
+   */
+  private val _key = Property.StickyKey[UnusedData]
+
+  class PostTyper extends CheckUnused(PhaseMode.Aggregate, "PostTyper", _key)
+
+  class PostInlining extends CheckUnused(PhaseMode.Report, "PostInlining", _key)
+
   /**
    * A stateful class gathering the infos on :
    * - imports
    * - definitions
    * - usage
    */
   private class UnusedData:
-    import dotty.tools.dotc.transform.CheckUnused.UnusedData.UnusedResult
     import collection.mutable.{Set => MutSet, Map => MutMap, Stack => MutStack}
-    import UnusedData.ScopeType
+    import UnusedData.*
 
     /** The current scope during the tree traversal */
-    var currScopeType: MutStack[ScopeType] = MutStack(ScopeType.Other)
+    val currScopeType: MutStack[ScopeType] = MutStack(ScopeType.Other)
+
+    var unusedAggregate: Option[UnusedResult] = None
 
     /* IMPORTS */
     private val impInScope = MutStack(MutSet[tpd.Import]())
@@ -344,6 +360,17 @@ object CheckUnused:
       execInNewScope
       popScope()
 
+    def finishAggregation(using Context)(): Unit =
+      val unusedInThisStage = this.getUnused
+      this.unusedAggregate match {
+        case None =>
+          this.unusedAggregate = Some(unusedInThisStage)
+        case Some(prevUnused) =>
+          val intersection = unusedInThisStage.warnings.intersect(prevUnused.warnings)
+          this.unusedAggregate = Some(UnusedResult(intersection))
+      }
+
+
     /**
      * Register a found (used) symbol along with its name
      *
@@ -453,12 +480,13 @@ object CheckUnused:
      *
      * The given `List` is sorted by line and then column of the position
      */
+
     def getUnused(using Context): UnusedResult =
       popScope()
 
       val sortedImp =
         if ctx.settings.WunusedHas.imports || ctx.settings.WunusedHas.strictNoImplicitWarn then
-          unusedImport.map(d => d.srcPos -> WarnTypes.Imports).toList
+          unusedImport.map(d => UnusedSymbol(d.srcPos, d.name, WarnTypes.Imports)).toList
         else
           Nil
       val sortedLocalDefs =
@@ -467,7 +495,7 @@ object CheckUnused:
             .filterNot(d => d.symbol.usedDefContains)
             .filterNot(d => usedInPosition.exists { case (pos, name) => d.span.contains(pos.span) && name == d.symbol.name})
             .filterNot(d => containsSyntheticSuffix(d.symbol))
-            .map(d => d.namePos -> WarnTypes.LocalDefs).toList
+            .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.LocalDefs)).toList
         else
           Nil
       val sortedExplicitParams =
@@ -476,23 +504,23 @@ object CheckUnused:
             .filterNot(d => d.symbol.usedDefContains)
             .filterNot(d => usedInPosition.exists { case (pos, name) => d.span.contains(pos.span) && name == d.symbol.name})
             .filterNot(d => containsSyntheticSuffix(d.symbol))
-            .map(d => d.namePos -> WarnTypes.ExplicitParams).toList
+            .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.ExplicitParams)).toList
         else
           Nil
       val sortedImplicitParams =
         if ctx.settings.WunusedHas.implicits then
           implicitParamInScope
             .filterNot(d => d.symbol.usedDefContains)
             .filterNot(d => containsSyntheticSuffix(d.symbol))
-            .map(d => d.namePos -> WarnTypes.ImplicitParams).toList
+            .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.ImplicitParams)).toList
         else
           Nil
       val sortedPrivateDefs =
         if ctx.settings.WunusedHas.privates then
           privateDefInScope
             .filterNot(d => d.symbol.usedDefContains)
             .filterNot(d => containsSyntheticSuffix(d.symbol))
-            .map(d => d.namePos -> WarnTypes.PrivateMembers).toList
+            .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.PrivateMembers)).toList
         else
           Nil
       val sortedPatVars =
@@ -501,14 +529,14 @@ object CheckUnused:
             .filterNot(d => d.symbol.usedDefContains)
             .filterNot(d => containsSyntheticSuffix(d.symbol))
             .filterNot(d => usedInPosition.exists { case (pos, name) => d.span.contains(pos.span) && name == d.symbol.name})
-            .map(d => d.namePos -> WarnTypes.PatVars).toList
+            .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.PatVars)).toList
         else
           Nil
       val warnings = List(sortedImp, sortedLocalDefs, sortedExplicitParams, sortedImplicitParams, sortedPrivateDefs, sortedPatVars).flatten.sortBy { s =>
-        val pos = s._1.sourcePos
+        val pos = s.pos.sourcePos
         (pos.line, pos.column)
       }
-      UnusedResult(warnings, Nil)
+      UnusedResult(warnings.toSet)
     end getUnused
     //============================ HELPERS ====================================
 
@@ -707,7 +735,11 @@ object CheckUnused:
           case _:tpd.Block => Local
           case _ => Other
 
+      case class UnusedSymbol(pos: SrcPos, name: Name, warnType: WarnTypes)
       /** A container for the results of the used elements analysis */
-      case class UnusedResult(warnings: List[(dotty.tools.dotc.util.SrcPos, WarnTypes)], usedImports: List[(tpd.Import, untpd.ImportSelector)])
+      case class UnusedResult(warnings: Set[UnusedSymbol])
+      object UnusedResult:
+        val Empty = UnusedResult(Set.empty)
+
 end CheckUnused
 

tests/neg-custom-args/fatal-warnings/i15503a.scala

@@ -63,12 +63,12 @@ object FooTypeName:
 
 object InlineChecks:
   object InlineFoo:
-    import collection.mutable.Set // OK
+    import collection.mutable.Set // ok
     import collection.mutable.Map // error
     inline def getSet = Set(1)
 
   object InlinedBar:
-    import collection.mutable.Set // error
+    import collection.mutable.Set // ok
     import collection.mutable.Map // error
     val a = InlineFoo.getSet
 

tests/neg-custom-args/fatal-warnings/i16876/Macro.scala

@@ -0,0 +1,23 @@
+import scala.quoted.*
+
+def findMethodSymbol(using q: Quotes)(s: quotes.reflect.Symbol): quotes.reflect.Symbol =
+  if s.isDefDef then
+    s
+  else
+    findMethodSymbol(using q)(s.owner)
+end findMethodSymbol
+
+
+inline def adder: Int = ${
+  adderImpl
+}
+
+def adderImpl(using q: Quotes): Expr[Int] =
+  import quotes.reflect.*
+
+  val inputs = findMethodSymbol(using q)(q.reflect.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 })

tests/neg-custom-args/fatal-warnings/i16876/Test.scala

@@ -0,0 +1,11 @@
+// scalac: -Wunused:all
+
+object Foo {
+  private def myMethod(a: Int, b: Int, c: Int) = adder // ok
+  myMethod(1, 2, 3)
+
+  private def myMethodFailing(a: Int, b: Int, c: Int) = a + 0 // error // error
+  myMethodFailing(1, 2, 3)
+}
+
+