Skip to content

Commit 5328eff

Browse files
Backport "Remove premature caching of lookups for unused lint" to 3.7.1 (#23227)
Backports #22982 and #23069 to the 3.7.1-RC2. PR submitted by the release tooling.
2 parents aede144 + 2c050aa commit 5328eff

File tree

3 files changed

+68
-78
lines changed

3 files changed

+68
-78
lines changed

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

Lines changed: 9 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -215,15 +215,6 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
215215
refInfos.register(tree)
216216
tree
217217

218-
override def prepareForTemplate(tree: Template)(using Context): Context =
219-
ctx.fresh.setProperty(resolvedKey, Resolved())
220-
221-
override def prepareForPackageDef(tree: PackageDef)(using Context): Context =
222-
ctx.fresh.setProperty(resolvedKey, Resolved())
223-
224-
override def prepareForStats(trees: List[Tree])(using Context): Context =
225-
ctx.fresh.setProperty(resolvedKey, Resolved())
226-
227218
override def transformOther(tree: Tree)(using Context): tree.type =
228219
tree match
229220
case imp: Import =>
@@ -289,7 +280,6 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
289280
case ByNameTypeTree(result) =>
290281
transformAllDeep(result)
291282
//case _: InferredTypeTree => // do nothing
292-
//case _: Export => // nothing to do
293283
//case _ if tree.isType =>
294284
case _ =>
295285
tree
@@ -350,15 +340,6 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
350340
&& ctxsym.thisType.baseClasses.contains(sym.owner)
351341
&& ctxsym.thisType.member(sym.name).hasAltWith(d => d.containsSym(sym) && !name.exists(_ != d.name))
352342

353-
// Attempt to cache a result at the given context. Not all contexts bear a cache, including NoContext.
354-
// If there is already any result for the name and prefix, do nothing.
355-
def addCached(where: Context, result: Precedence): Unit =
356-
if where.moreProperties ne null then
357-
where.property(resolvedKey) match
358-
case Some(resolved) =>
359-
resolved.record(sym, name, prefix, result)
360-
case none =>
361-
362343
// Avoid spurious NoSymbol and also primary ctors which are never warned about.
363344
// Selections C.this.toString should be already excluded, but backtopped here for eq, etc.
364345
if !sym.exists || sym.isPrimaryConstructor || sym.isEffectiveRoot || defn.topClasses(sym.owner) then return
@@ -367,39 +348,20 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
367348
// If the sym is an enclosing definition (the owner of a context), it does not count toward usages.
368349
val isLocal = sym.isLocalToBlock
369350
var candidate: Context = NoContext
370-
var cachePoint: Context = NoContext // last context with Resolved cache
371351
var importer: ImportSelector | Null = null // non-null for import context
372352
var precedence = NoPrecedence // of current resolution
353+
var enclosed = false // true if sym is owner of an enclosing context
373354
var done = false
374-
var cached = false
375355
val ctxs = ctx.outersIterator
376356
while !done && ctxs.hasNext do
377357
val cur = ctxs.next()
378-
if cur.owner eq sym then
379-
addCached(cachePoint, Definition)
380-
return // found enclosing definition
381-
else if isLocal then
358+
if cur.owner.userSymbol == sym && !sym.is(Package) then
359+
enclosed = true // found enclosing definition, don't register the reference
360+
if isLocal then
382361
if cur.owner eq sym.owner then
383362
done = true // for local def, just checking that it is not enclosing
384363
else
385-
val cachedPrecedence =
386-
cur.property(resolvedKey) match
387-
case Some(resolved) =>
388-
// conservative, cache must be nested below the result context
389-
if precedence.isNone then
390-
cachePoint = cur // no result yet, and future result could be cached here
391-
resolved.hasRecord(sym, name, prefix)
392-
case none => NoPrecedence
393-
cached = !cachedPrecedence.isNone
394-
if cached then
395-
// if prefer cached precedence, then discard previous result
396-
if precedence.weakerThan(cachedPrecedence) then
397-
candidate = NoContext
398-
importer = null
399-
cachePoint = cur // actual cache context
400-
precedence = cachedPrecedence // actual cached precedence
401-
done = true
402-
else if cur.isImportContext then
364+
if cur.isImportContext then
403365
val sel = matchingSelector(cur.importInfo.nn)
404366
if sel != null then
405367
if cur.importInfo.nn.isRootImport then
@@ -419,7 +381,7 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
419381
candidate = cur
420382
importer = sel
421383
else if checkMember(cur.owner) then
422-
if sym.srcPos.sourcePos.source == ctx.source then
384+
if sym.is(Package) || sym.srcPos.sourcePos.source == ctx.source then
423385
precedence = Definition
424386
candidate = cur
425387
importer = null // ignore import in same scope; we can't check nesting level
@@ -429,16 +391,10 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
429391
candidate = cur
430392
end while
431393
// record usage and possibly an import
432-
refInfos.refs.addOne(sym)
394+
if !enclosed then
395+
refInfos.refs.addOne(sym)
433396
if candidate != NoContext && candidate.isImportContext && importer != null then
434397
refInfos.sels.put(importer, ())
435-
// possibly record that we have performed this look-up
436-
// if no result was found, take it as Definition (local or rooted head of fully qualified path)
437-
val adjusted = if precedence.isNone then Definition else precedence
438-
if !cached && (cachePoint ne NoContext) then
439-
addCached(cachePoint, adjusted)
440-
if cachePoint ne ctx then
441-
addCached(ctx, adjusted) // at this ctx, since cachePoint may be far up the outer chain
442398
end resolveUsage
443399
end CheckUnused
444400

@@ -450,15 +406,8 @@ object CheckUnused:
450406

451407
val refInfosKey = Property.StickyKey[RefInfos]
452408

453-
val resolvedKey = Property.Key[Resolved]
454-
455409
inline def refInfos(using Context): RefInfos = ctx.property(refInfosKey).get
456410

457-
inline def resolved(using Context): Resolved =
458-
ctx.property(resolvedKey) match
459-
case Some(res) => res
460-
case _ => throw new MatchError("no Resolved for context")
461-
462411
/** Attachment holding the name of an Ident as written by the user. */
463412
val OriginalName = Property.StickyKey[Name]
464413

@@ -487,7 +436,7 @@ object CheckUnused:
487436
if inliners == 0
488437
&& languageImport(imp.expr).isEmpty
489438
&& !imp.isGeneratedByEnum
490-
&& !ctx.outer.owner.name.isReplWrapperName
439+
&& !ctx.owner.name.isReplWrapperName
491440
then
492441
imps.put(imp, ())
493442
case tree: Bind =>
@@ -514,24 +463,6 @@ object CheckUnused:
514463
var inliners = 0 // depth of inline def (not inlined yet)
515464
end RefInfos
516465

517-
// Symbols already resolved in the given Context (with name and prefix of lookup).
518-
class Resolved:
519-
import PrecedenceLevels.*
520-
private val seen = mutable.Map.empty[Symbol, List[(Name, Type, Precedence)]].withDefaultValue(Nil)
521-
// if a result has been recorded, return it; otherwise, NoPrecedence.
522-
def hasRecord(symbol: Symbol, name: Name, prefix: Type)(using Context): Precedence =
523-
seen(symbol).find((n, p, _) => n == name && p =:= prefix) match
524-
case Some((_, _, r)) => r
525-
case none => NoPrecedence
526-
// "record" the look-up result, if there is not already a result for the name and prefix.
527-
def record(symbol: Symbol, name: Name, prefix: Type, result: Precedence)(using Context): Unit =
528-
require(NoPrecedence.weakerThan(result))
529-
seen.updateWith(symbol):
530-
case svs @ Some(vs) =>
531-
if vs.exists((n, p, _) => n == name && p =:= prefix) then svs
532-
else Some((name, prefix, result) :: vs)
533-
case none => Some((name, prefix, result) :: Nil)
534-
535466
// Names are resolved by definitions and imports, which have four precedence levels:
536467
object PrecedenceLevels:
537468
opaque type Precedence = Int

tests/warn/i22971.scala

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
//> using options -Wunused:imports
2+
3+
package p:
4+
5+
trait Base
6+
class Class extends Base
7+
8+
abstract class Entity[T: GetType]
9+
10+
class Thing extends Entity[Class]
11+
12+
trait GetType[T]
13+
14+
object GetType {
15+
//implicit object GetTypeClass extends GetType[Class]
16+
implicit val GetTypeClass: GetType[Class] = new GetType[Class] {}
17+
}
18+
object Main {
19+
def main(args: Array[String]): Unit = {
20+
import GetType.*
21+
val e = GetTypeClass
22+
}
23+
}
24+
25+
package q:
26+
27+
class C:
28+
def f =
29+
import p.*
30+
GetType.GetTypeClass
31+
def g =
32+
import p.GetType.*
33+
GetTypeClass
34+
class D extends p.Entity[p.Class]

tests/warn/i23047.scala

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
//> using options -Wunused:imports
2+
3+
package some.example:
4+
package demo:
5+
6+
import some.example // no warn because enclosing package example is not available as a simple name in some
7+
8+
object Main {
9+
10+
def generic[T](x: Any): T = null.asInstanceOf[T]
11+
12+
def main(args: Array[String]): Unit = {
13+
generic[example.Util](0)
14+
15+
import some.example.demo.Main // warn
16+
println(Main)
17+
18+
import some.example.demo // warn because enclosing package demo is available as a simple name
19+
println(demo.Main)
20+
}
21+
}
22+
23+
package some.example:
24+
25+
class Util

0 commit comments

Comments
 (0)