Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport "Make CheckUnused run both after Typer and Inlining" #17279

Merged
merged 6 commits into from
Apr 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
100 changes: 66 additions & 34 deletions compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 ==========
Expand Down Expand Up @@ -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
Expand All @@ -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]())
Expand Down Expand Up @@ -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
*
Expand Down Expand Up @@ -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 =
Expand All @@ -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 =
Expand All @@ -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 =
Expand All @@ -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 ====================================

Expand Down Expand Up @@ -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

4 changes: 2 additions & 2 deletions tests/neg-custom-args/fatal-warnings/i15503a.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
23 changes: 23 additions & 0 deletions tests/neg-custom-args/fatal-warnings/i16876/Macro.scala
Original file line number Diff line number Diff line change
@@ -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 })
11 changes: 11 additions & 0 deletions tests/neg-custom-args/fatal-warnings/i16876/Test.scala
Original file line number Diff line number Diff line change
@@ -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)
}