From 2e471146fb40789cc8d529337553fcc1ec0e78ab Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 30 Apr 2025 07:43:25 -0700 Subject: [PATCH 1/3] REPL wrapper is not ctx.outer [Cherry-picked c5d5214f9b0955574d594abe256762932ed7589e] --- compiler/src/dotty/tools/dotc/transform/CheckUnused.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index fafb8ea72c1c..eeebf7c28b82 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -487,7 +487,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 => From c504b7d87fd6a523a5b8a837ec49ba948fa71c0a Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 29 Apr 2025 20:55:01 -0700 Subject: [PATCH 2/3] Remove premature, brittle caching in lint --- .../tools/dotc/transform/CheckUnused.scala | 72 +------------------ tests/warn/i22971.scala | 34 +++++++++ 2 files changed, 35 insertions(+), 71 deletions(-) create mode 100644 tests/warn/i22971.scala diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index eeebf7c28b82..036945cdff8e 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -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 => @@ -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 @@ -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 @@ -367,11 +348,9 @@ 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 done = false - var cached = false val ctxs = ctx.outersIterator while !done && ctxs.hasNext do val cur = ctxs.next() @@ -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 @@ -432,13 +394,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 @@ -450,15 +405,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] @@ -514,24 +462,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 diff --git a/tests/warn/i22971.scala b/tests/warn/i22971.scala new file mode 100644 index 000000000000..63eee06a8c52 --- /dev/null +++ b/tests/warn/i22971.scala @@ -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] From 2c050aafc487701f1308101221f1b242ecb9ee71 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 29 Apr 2025 14:09:05 -0700 Subject: [PATCH 3/3] Enclosing package p.q not visible as q --- .../tools/dotc/transform/CheckUnused.scala | 13 +++++----- tests/warn/i23047.scala | 25 +++++++++++++++++++ 2 files changed, 32 insertions(+), 6 deletions(-) create mode 100644 tests/warn/i23047.scala diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 036945cdff8e..99e9f4d53db7 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -350,14 +350,14 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha var candidate: Context = NoContext 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 val ctxs = ctx.outersIterator while !done && ctxs.hasNext do val cur = ctxs.next() - if cur.owner eq sym then - addCached(cachePoint, Definition) - return // found enclosing definition - else if isLocal then + if cur.owner.userSymbol == sym && !sym.is(Package) then + enclosed = true // found enclosing definition, don't register the reference + if isLocal then if cur.owner eq sym.owner then done = true // for local def, just checking that it is not enclosing else @@ -381,7 +381,7 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha candidate = cur importer = sel else if checkMember(cur.owner) then - if sym.srcPos.sourcePos.source == ctx.source then + if sym.is(Package) || sym.srcPos.sourcePos.source == ctx.source then precedence = Definition candidate = cur importer = null // ignore import in same scope; we can't check nesting level @@ -391,7 +391,8 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha candidate = cur end while // record usage and possibly an import - refInfos.refs.addOne(sym) + if !enclosed then + refInfos.refs.addOne(sym) if candidate != NoContext && candidate.isImportContext && importer != null then refInfos.sels.put(importer, ()) end resolveUsage diff --git a/tests/warn/i23047.scala b/tests/warn/i23047.scala new file mode 100644 index 000000000000..1922797c3185 --- /dev/null +++ b/tests/warn/i23047.scala @@ -0,0 +1,25 @@ +//> using options -Wunused:imports + +package some.example: + package demo: + + import some.example // no warn because enclosing package example is not available as a simple name in some + + object Main { + + def generic[T](x: Any): T = null.asInstanceOf[T] + + def main(args: Array[String]): Unit = { + generic[example.Util](0) + + import some.example.demo.Main // warn + println(Main) + + import some.example.demo // warn because enclosing package demo is available as a simple name + println(demo.Main) + } + } + +package some.example: + + class Util