Skip to content

Remove premature caching of lookups for unused lint #22982

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

Merged
merged 2 commits into from
May 7, 2025
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
74 changes: 2 additions & 72 deletions compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
Original file line number Diff line number Diff line change
Expand Up @@ -215,15 +215,6 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
refInfos.register(tree)
tree

override def prepareForTemplate(tree: Template)(using Context): Context =
ctx.fresh.setProperty(resolvedKey, Resolved())

override def prepareForPackageDef(tree: PackageDef)(using Context): Context =
ctx.fresh.setProperty(resolvedKey, Resolved())

override def prepareForStats(trees: List[Tree])(using Context): Context =
ctx.fresh.setProperty(resolvedKey, Resolved())

override def transformOther(tree: Tree)(using Context): tree.type =
tree match
case imp: Import =>
Expand Down Expand Up @@ -289,7 +280,6 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
case ByNameTypeTree(result) =>
transformAllDeep(result)
//case _: InferredTypeTree => // do nothing
//case _: Export => // nothing to do
//case _ if tree.isType =>
case _ =>
tree
Expand Down Expand Up @@ -350,15 +340,6 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
&& ctxsym.thisType.baseClasses.contains(sym.owner)
&& ctxsym.thisType.member(sym.name).hasAltWith(d => d.containsSym(sym) && !name.exists(_ != d.name))

// Attempt to cache a result at the given context. Not all contexts bear a cache, including NoContext.
// If there is already any result for the name and prefix, do nothing.
def addCached(where: Context, result: Precedence): Unit =
if where.moreProperties ne null then
where.property(resolvedKey) match
case Some(resolved) =>
resolved.record(sym, name, prefix, result)
case none =>

// Avoid spurious NoSymbol and also primary ctors which are never warned about.
// Selections C.this.toString should be already excluded, but backtopped here for eq, etc.
if !sym.exists || sym.isPrimaryConstructor || sym.isEffectiveRoot || defn.topClasses(sym.owner) then return
Expand All @@ -367,12 +348,10 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
// If the sym is an enclosing definition (the owner of a context), it does not count toward usages.
val isLocal = sym.isLocalToBlock
var candidate: Context = NoContext
var cachePoint: Context = NoContext // last context with Resolved cache
var importer: ImportSelector | Null = null // non-null for import context
var precedence = NoPrecedence // of current resolution
var enclosed = false // true if sym is owner of an enclosing context
var done = false
var cached = false
val ctxs = ctx.outersIterator
while !done && ctxs.hasNext do
val cur = ctxs.next()
Expand All @@ -382,24 +361,7 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
if cur.owner eq sym.owner then
done = true // for local def, just checking that it is not enclosing
else
val cachedPrecedence =
cur.property(resolvedKey) match
case Some(resolved) =>
// conservative, cache must be nested below the result context
if precedence.isNone then
cachePoint = cur // no result yet, and future result could be cached here
resolved.hasRecord(sym, name, prefix)
case none => NoPrecedence
cached = !cachedPrecedence.isNone
if cached then
// if prefer cached precedence, then discard previous result
if precedence.weakerThan(cachedPrecedence) then
candidate = NoContext
importer = null
cachePoint = cur // actual cache context
precedence = cachedPrecedence // actual cached precedence
done = true
else if cur.isImportContext then
if cur.isImportContext then
val sel = matchingSelector(cur.importInfo.nn)
if sel != null then
if cur.importInfo.nn.isRootImport then
Expand Down Expand Up @@ -433,13 +395,6 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
refInfos.refs.addOne(sym)
if candidate != NoContext && candidate.isImportContext && importer != null then
refInfos.sels.put(importer, ())
// possibly record that we have performed this look-up
// if no result was found, take it as Definition (local or rooted head of fully qualified path)
val adjusted = if precedence.isNone then Definition else precedence
if !cached && (cachePoint ne NoContext) then
addCached(cachePoint, adjusted)
if cachePoint ne ctx then
addCached(ctx, adjusted) // at this ctx, since cachePoint may be far up the outer chain
end resolveUsage
end CheckUnused

Expand All @@ -451,15 +406,8 @@ object CheckUnused:

val refInfosKey = Property.StickyKey[RefInfos]

val resolvedKey = Property.Key[Resolved]

inline def refInfos(using Context): RefInfos = ctx.property(refInfosKey).get

inline def resolved(using Context): Resolved =
ctx.property(resolvedKey) match
case Some(res) => res
case _ => throw new MatchError("no Resolved for context")

/** Attachment holding the name of an Ident as written by the user. */
val OriginalName = Property.StickyKey[Name]

Expand Down Expand Up @@ -488,7 +436,7 @@ object CheckUnused:
if inliners == 0
&& languageImport(imp.expr).isEmpty
&& !imp.isGeneratedByEnum
&& !ctx.outer.owner.name.isReplWrapperName
&& !ctx.owner.name.isReplWrapperName
then
imps.put(imp, ())
case tree: Bind =>
Expand All @@ -515,24 +463,6 @@ object CheckUnused:
var inliners = 0 // depth of inline def (not inlined yet)
end RefInfos

// Symbols already resolved in the given Context (with name and prefix of lookup).
class Resolved:
import PrecedenceLevels.*
private val seen = mutable.Map.empty[Symbol, List[(Name, Type, Precedence)]].withDefaultValue(Nil)
// if a result has been recorded, return it; otherwise, NoPrecedence.
def hasRecord(symbol: Symbol, name: Name, prefix: Type)(using Context): Precedence =
seen(symbol).find((n, p, _) => n == name && p =:= prefix) match
case Some((_, _, r)) => r
case none => NoPrecedence
// "record" the look-up result, if there is not already a result for the name and prefix.
def record(symbol: Symbol, name: Name, prefix: Type, result: Precedence)(using Context): Unit =
require(NoPrecedence.weakerThan(result))
seen.updateWith(symbol):
case svs @ Some(vs) =>
if vs.exists((n, p, _) => n == name && p =:= prefix) then svs
else Some((name, prefix, result) :: vs)
case none => Some((name, prefix, result) :: Nil)

// Names are resolved by definitions and imports, which have four precedence levels:
object PrecedenceLevels:
opaque type Precedence = Int
Expand Down
34 changes: 34 additions & 0 deletions tests/warn/i22971.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//> using options -Wunused:imports

package p:

trait Base
class Class extends Base

abstract class Entity[T: GetType]

class Thing extends Entity[Class]

trait GetType[T]

object GetType {
//implicit object GetTypeClass extends GetType[Class]
implicit val GetTypeClass: GetType[Class] = new GetType[Class] {}
}
object Main {
def main(args: Array[String]): Unit = {
import GetType.*
val e = GetTypeClass
}
}

package q:

class C:
def f =
import p.*
GetType.GetTypeClass
def g =
import p.GetType.*
GetTypeClass
class D extends p.Entity[p.Class]
Loading