From 69664f7c01e0103cc02ee5fae9d63ad04abe46d7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= <sjrdoeraene@gmail.com>
Date: Thu, 2 May 2024 13:47:40 +0200
Subject: [PATCH 01/10] Make `unusedDataApply` inline so that no closure is
 allocated.

It is used for every single tree in `CheckUnused`, so this is worth
it.
---
 compiler/src/dotty/tools/dotc/transform/CheckUnused.scala | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
index deb8446affbb..eece239e93f4 100644
--- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
+++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
@@ -40,8 +40,10 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
   import CheckUnused.*
   import UnusedData.*
 
-  private def unusedDataApply[U](f: UnusedData => U)(using Context): Context =
-    ctx.property(_key).foreach(f)
+  private inline def unusedDataApply[U](inline f: UnusedData => U)(using Context): Context =
+    ctx.property(_key) match
+      case Some(ud) => f(ud)
+      case None     => ()
     ctx
 
   override def phaseName: String = CheckUnused.phaseNamePrefix + suffix

From 0bd4b156aed129c141627283edeaddc518989a31 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= <sjrdoeraene@gmail.com>
Date: Thu, 2 May 2024 13:49:16 +0200
Subject: [PATCH 02/10] Do not use `LazyList` in `CheckUnused`.

It is not efficient when the results are always used exactly once.
---
 .../tools/dotc/transform/CheckUnused.scala     | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
index eece239e93f4..8dbedd5c69f6 100644
--- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
+++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
@@ -1,5 +1,7 @@
 package dotty.tools.dotc.transform
 
+import scala.annotation.tailrec
+
 import dotty.tools.dotc.ast.tpd
 import dotty.tools.dotc.core.Symbols.*
 import dotty.tools.dotc.ast.tpd.{Inlined, TreeTraverser}
@@ -88,11 +90,17 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
 
   override def prepareForIdent(tree: tpd.Ident)(using Context): Context =
     if tree.symbol.exists then
-      val prefixes = LazyList.iterate(tree.typeOpt.normalizedPrefix)(_.normalizedPrefix).takeWhile(_ != NoType)
-        .take(10) // Failsafe for the odd case if there was an infinite cycle
-      for prefix <- prefixes do
-        unusedDataApply(_.registerUsed(prefix.classSymbol, None))
-      unusedDataApply(_.registerUsed(tree.symbol, Some(tree.name)))
+      unusedDataApply { ud =>
+        @tailrec
+        def loopOnNormalizedPrefixes(prefix: Type, depth: Int): Unit =
+          // limit to 10 as failsafe for the odd case where there is an infinite cycle
+          if depth < 10 && prefix.exists then
+            ud.registerUsed(prefix.classSymbol, None)
+            loopOnNormalizedPrefixes(prefix.normalizedPrefix, depth + 1)
+
+        loopOnNormalizedPrefixes(tree.typeOpt.normalizedPrefix, depth = 0)
+        ud.registerUsed(tree.symbol, Some(tree.name))
+      }
     else if tree.hasType then
       unusedDataApply(_.registerUsed(tree.tpe.classSymbol, Some(tree.name)))
     else

From 6d79caa74fa2137261607cda802d75213c3e4370 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= <sjrdoeraene@gmail.com>
Date: Thu, 2 May 2024 13:51:13 +0200
Subject: [PATCH 03/10] Do not mangle names only to test whether it starts with
 a given string.

---
 compiler/src/dotty/tools/dotc/transform/CheckUnused.scala | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
index 8dbedd5c69f6..7741565d11a2 100644
--- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
+++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
@@ -125,8 +125,7 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
       traverseAnnotations(tree.symbol)
       if !tree.symbol.is(Module) then
         ud.registerDef(tree)
-      if tree.name.mangledString.startsWith(nme.derived.mangledString + "$")
-          && tree.typeOpt != NoType then
+      if tree.name.startsWith("derived$") && tree.typeOpt != NoType then
         ud.registerUsed(tree.typeOpt.typeSymbol, None, true)
       ud.addIgnoredUsage(tree.symbol)
     }

From 3188177a494877241f973efe557bb6e6d6c6d358 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= <sjrdoeraene@gmail.com>
Date: Thu, 2 May 2024 13:51:43 +0200
Subject: [PATCH 04/10] Remove dead code `newCtx` in `CheckUnused`.

---
 compiler/src/dotty/tools/dotc/transform/CheckUnused.scala | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
index 7741565d11a2..31fcc14d646d 100644
--- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
+++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
@@ -207,9 +207,6 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
     }
     ctx
 
-  private def newCtx(tree: tpd.Tree)(using Context) =
-    if tree.symbol.exists then ctx.withOwner(tree.symbol) else ctx
-
   /**
    * This traverse is the **main** component of this phase
    *

From a55ee4d5f3b0db7dc4b95b2beb6d4306395db0e6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= <sjrdoeraene@gmail.com>
Date: Thu, 2 May 2024 13:54:29 +0200
Subject: [PATCH 05/10] Do not use Sets of Trees in `CheckUnused`.

`Tree`s have structural equality. Even if `==` should be able to
exit quickly either because of `eq` or an early difference, sets
systematically call `hashCode`, which is going to recurse into the
entire structure.
---
 .../tools/dotc/transform/CheckUnused.scala    | 66 +++++++++++--------
 1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
index 31fcc14d646d..12f927194300 100644
--- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
+++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
@@ -29,6 +29,7 @@ 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 dotty.tools.dotc.util.Spans.Span
 import scala.math.Ordering
 
 
@@ -365,16 +366,16 @@ object CheckUnused:
      * See the `isAccessibleAsIdent` extension method below in the file
      */
     private val usedInScope = MutStack(MutSet[(Symbol,Boolean, Option[Name], Boolean)]())
-    private val usedInPosition = MutSet[(SrcPos, Name)]()
+    private val usedInPosition = MutMap.empty[Name, MutSet[Symbol]]
     /* unused import collected during traversal */
-    private val unusedImport = MutSet[ImportSelector]()
+    private val unusedImport = new java.util.IdentityHashMap[ImportSelector, Unit]
 
     /* LOCAL DEF OR VAL / Private Def or Val / Pattern variables */
-    private val localDefInScope = MutSet[tpd.MemberDef]()
-    private val privateDefInScope = MutSet[tpd.MemberDef]()
-    private val explicitParamInScope = MutSet[tpd.MemberDef]()
-    private val implicitParamInScope = MutSet[tpd.MemberDef]()
-    private val patVarsInScope = MutSet[tpd.Bind]()
+    private val localDefInScope = MutList.empty[tpd.MemberDef]
+    private val privateDefInScope = MutList.empty[tpd.MemberDef]
+    private val explicitParamInScope = MutList.empty[tpd.MemberDef]
+    private val implicitParamInScope = MutList.empty[tpd.MemberDef]
+    private val patVarsInScope = MutList.empty[tpd.Bind]
 
     /** All variables sets*/
     private val setVars = MutSet[Symbol]()
@@ -416,7 +417,8 @@ object CheckUnused:
           usedInScope.top += ((sym.companionModule, sym.isAccessibleAsIdent, name, isDerived))
           usedInScope.top += ((sym.companionClass, sym.isAccessibleAsIdent, name, isDerived))
           if sym.sourcePos.exists then
-            name.map(n => usedInPosition += ((sym.sourcePos, n)))
+            for n <- name do
+              usedInPosition.getOrElseUpdate(n, MutSet.empty) += sym
 
     /** Register a symbol that should be ignored */
     def addIgnoredUsage(sym: Symbol)(using Context): Unit =
@@ -434,9 +436,9 @@ object CheckUnused:
       if !tpd.languageImport(imp.expr).nonEmpty && !imp.isGeneratedByEnum && !isTransparentAndInline(imp) then
         impInScope.top += imp
         if currScopeType.top != ScopeType.ReplWrapper then // #18383 Do not report top-level import's in the repl as unused
-          unusedImport ++= imp.selectors.filter { s =>
-            !shouldSelectorBeReported(imp, s) && !isImportExclusion(s) && !isImportIgnored(imp, s)
-          }
+          for s <- imp.selectors do
+            if !shouldSelectorBeReported(imp, s) && !isImportExclusion(s) && !isImportIgnored(imp, s) then
+              unusedImport.put(s, ())
     end registerImport
 
     /** Register (or not) some `val` or `def` according to the context, scope and flags */
@@ -491,11 +493,11 @@ object CheckUnused:
               // We keep wildcard symbol for the end as they have the least precedence
               false
             case Some(sel) =>
-              unusedImport -= sel
+              unusedImport.remove(sel)
               true
         }
         if !matchedExplicitImport && selWildCard.isDefined then
-          unusedImport -= selWildCard.get
+          unusedImport.remove(selWildCard.get)
           true // a matching import exists so the symbol won't be kept for outer scope
         else
           matchedExplicitImport
@@ -520,56 +522,64 @@ object CheckUnused:
 
     def getUnused(using Context): UnusedResult =
       popScope()
+
+      def isUsedInPosition(name: Name, span: Span): Boolean =
+        usedInPosition.get(name) match
+          case Some(syms) => syms.exists(sym => span.contains(sym.span))
+          case None       => false
+
       val sortedImp =
         if ctx.settings.WunusedHas.imports || ctx.settings.WunusedHas.strictNoImplicitWarn then
-          unusedImport.map(d => UnusedSymbol(d.srcPos, d.name, WarnTypes.Imports)).toList
+          import scala.jdk.CollectionConverters.*
+          unusedImport.keySet().nn.iterator().nn.asScala
+            .map(d => UnusedSymbol(d.srcPos, d.name, WarnTypes.Imports)).toList
         else
           Nil
       // Partition to extract unset local variables from usedLocalDefs
       val (usedLocalDefs, unusedLocalDefs) =
         if ctx.settings.WunusedHas.locals then
-          localDefInScope.partition(d => d.symbol.usedDefContains)
+          localDefInScope.toList.partition(d => d.symbol.usedDefContains)
         else
           (Nil, Nil)
       val sortedLocalDefs =
         unusedLocalDefs
-          .filterNot(d => usedInPosition.exists { case (pos, name) => d.span.contains(pos.span) && name == d.symbol.name})
+          .filterNot(d => isUsedInPosition(d.symbol.name, d.span))
           .filterNot(d => containsSyntheticSuffix(d.symbol))
-          .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.LocalDefs)).toList
+          .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.LocalDefs))
       val unsetLocalDefs = usedLocalDefs.filter(isUnsetVarDef).map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.UnsetLocals)).toList
 
       val sortedExplicitParams =
         if ctx.settings.WunusedHas.explicits then
-          explicitParamInScope
+          explicitParamInScope.toList
             .filterNot(d => d.symbol.usedDefContains)
-            .filterNot(d => usedInPosition.exists { case (pos, name) => d.span.contains(pos.span) && name == d.symbol.name})
+            .filterNot(d => isUsedInPosition(d.symbol.name, d.span))
             .filterNot(d => containsSyntheticSuffix(d.symbol))
-            .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.ExplicitParams)).toList
+            .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.ExplicitParams))
         else
           Nil
       val sortedImplicitParams =
         if ctx.settings.WunusedHas.implicits then
-          implicitParamInScope
+          implicitParamInScope.toList
             .filterNot(d => d.symbol.usedDefContains)
             .filterNot(d => containsSyntheticSuffix(d.symbol))
-            .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.ImplicitParams)).toList
+            .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.ImplicitParams))
         else
           Nil
       // Partition to extract unset private variables from usedPrivates
       val (usedPrivates, unusedPrivates) =
         if ctx.settings.WunusedHas.privates then
-          privateDefInScope.partition(d => d.symbol.usedDefContains)
+          privateDefInScope.toList.partition(d => d.symbol.usedDefContains)
         else
           (Nil, Nil)
-      val sortedPrivateDefs = unusedPrivates.filterNot(d => containsSyntheticSuffix(d.symbol)).map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.PrivateMembers)).toList
-      val unsetPrivateDefs = usedPrivates.filter(isUnsetVarDef).map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.UnsetPrivates)).toList
+      val sortedPrivateDefs = unusedPrivates.filterNot(d => containsSyntheticSuffix(d.symbol)).map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.PrivateMembers))
+      val unsetPrivateDefs = usedPrivates.filter(isUnsetVarDef).map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.UnsetPrivates))
       val sortedPatVars =
         if ctx.settings.WunusedHas.patvars then
-          patVarsInScope
+          patVarsInScope.toList
             .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 => UnusedSymbol(d.namePos, d.name, WarnTypes.PatVars)).toList
+            .filterNot(d => isUsedInPosition(d.symbol.name, d.span))
+            .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.PatVars))
         else
           Nil
       val warnings =

From 701d69fc2a5b4294d73357461331aaf5c2b06d72 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= <sjrdoeraene@gmail.com>
Date: Thu, 2 May 2024 13:56:00 +0200
Subject: [PATCH 06/10] Remove a useless sort and otherwise sort by offset, not
 line.

It is pointless to sort a list before converting it into a Set.
---
 .../tools/dotc/transform/CheckUnused.scala    | 23 ++++++++-----------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
index 12f927194300..d6c3cd394d5c 100644
--- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
+++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
@@ -290,7 +290,7 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
 
   /** Do the actual reporting given the result of the anaylsis */
   private def reportUnused(res: UnusedData.UnusedResult)(using Context): Unit =
-    res.warnings.toList.sortBy(_.pos.line)(using Ordering[Int]).foreach { s =>
+    res.warnings.toList.sortBy(_.pos.span.point)(using Ordering[Int]).foreach { s =>
       s match
         case UnusedSymbol(t, _, WarnTypes.Imports) =>
           report.warning(s"unused import", t)
@@ -583,19 +583,14 @@ object CheckUnused:
         else
           Nil
       val warnings =
-        val unsorted =
-          sortedImp :::
-          sortedLocalDefs :::
-          sortedExplicitParams :::
-          sortedImplicitParams :::
-          sortedPrivateDefs :::
-          sortedPatVars :::
-          unsetLocalDefs :::
-          unsetPrivateDefs
-        unsorted.sortBy { s =>
-          val pos = s.pos.sourcePos
-          (pos.line, pos.column)
-        }
+        sortedImp :::
+        sortedLocalDefs :::
+        sortedExplicitParams :::
+        sortedImplicitParams :::
+        sortedPrivateDefs :::
+        sortedPatVars :::
+        unsetLocalDefs :::
+        unsetPrivateDefs
       UnusedResult(warnings.toSet)
     end getUnused
     //============================ HELPERS ====================================

From 803dff70331589569f9707b01c78730ef16f8623 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= <sjrdoeraene@gmail.com>
Date: Fri, 3 May 2024 09:29:03 +0200
Subject: [PATCH 07/10] Refactor unused imports to try and make sense of it.

---
 .../tools/dotc/transform/CheckUnused.scala    | 76 +++++++++++--------
 1 file changed, 46 insertions(+), 30 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
index d6c3cd394d5c..ee80bdac5017 100644
--- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
+++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
@@ -10,6 +10,7 @@ import dotty.tools.dotc.ast.untpd.ImportSelector
 import dotty.tools.dotc.config.ScalaSettings
 import dotty.tools.dotc.core.Contexts.*
 import dotty.tools.dotc.core.Decorators.{em, i}
+import dotty.tools.dotc.core.Denotations.SingleDenotation
 import dotty.tools.dotc.core.Flags.*
 import dotty.tools.dotc.core.Phases.Phase
 import dotty.tools.dotc.core.StdNames
@@ -409,13 +410,18 @@ object CheckUnused:
      * as the same element can be imported with different renaming
      */
     def registerUsed(sym: Symbol, name: Option[Name], isDerived: Boolean = false)(using Context): Unit =
-      if !isConstructorOfSynth(sym) && !doNotRegister(sym) then
-        if sym.isConstructor && sym.exists then
+      if sym.exists && !isConstructorOfSynth(sym) && !doNotRegister(sym) then
+        if sym.isConstructor then
           registerUsed(sym.owner, None) // constructor are "implicitly" imported with the class
         else
-          usedInScope.top += ((sym, sym.isAccessibleAsIdent, name, isDerived))
-          usedInScope.top += ((sym.companionModule, sym.isAccessibleAsIdent, name, isDerived))
-          usedInScope.top += ((sym.companionClass, sym.isAccessibleAsIdent, name, isDerived))
+          val accessibleAsIdent = sym.isAccessibleAsIdent
+          def addIfExists(sym: Symbol): Unit =
+            if sym.exists then
+              usedDef += sym
+              usedInScope.top += ((sym, accessibleAsIdent, name, isDerived))
+          addIfExists(sym)
+          addIfExists(sym.companionModule)
+          addIfExists(sym.companionClass)
           if sym.sourcePos.exists then
             for n <- name do
               usedInPosition.getOrElseUpdate(n, MutSet.empty) += sym
@@ -508,8 +514,6 @@ object CheckUnused:
         // we keep the symbols not referencing an import in this scope
         // as it can be the only reference to an outer import
         usedInScope.top ++= kept
-      // register usage in this scope for other warnings at the end of the phase
-      usedDef ++= used.map(_._1)
       // retrieve previous scope type
       currScopeType.pop
     end popScope
@@ -685,42 +689,54 @@ object CheckUnused:
     extension (sym: Symbol)
       /** is accessible without import in current context */
       private def isAccessibleAsIdent(using Context): Boolean =
-        sym.exists &&
-          ctx.outersIterator.exists{ c =>
-            c.owner == sym.owner
-            || sym.owner.isClass && c.owner.isClass
-                && c.owner.thisType.baseClasses.contains(sym.owner)
-                && c.owner.thisType.member(sym.name).alternatives.contains(sym)
-          }
+        ctx.outersIterator.exists{ c =>
+          c.owner == sym.owner
+          || sym.owner.isClass && c.owner.isClass
+              && c.owner.thisType.baseClasses.contains(sym.owner)
+              && c.owner.thisType.member(sym.name).alternatives.contains(sym)
+        }
 
       /** Given an import and accessibility, return selector that matches import<->symbol */
       private def isInImport(imp: tpd.Import, isAccessible: Boolean, altName: Option[Name], isDerived: Boolean)(using Context): Option[ImportSelector] =
+        assert(sym.exists)
+
         val tpd.Import(qual, sels) = imp
         val qualTpe = qual.tpe
         val dealiasedSym = sym.dealias
-        val simpleSelections = qualTpe.member(sym.name).alternatives
-        val selectionsToDealias = sels.flatMap(sel =>
-          qualTpe.member(sel.name.toTypeName).alternatives
-          ::: qualTpe.member(sel.name.toTermName).alternatives)
-        def qualHasSymbol = simpleSelections.map(_.symbol).contains(sym) || (simpleSelections ::: selectionsToDealias).map(_.symbol).map(_.dealias).contains(dealiasedSym)
-        def selector = sels.find(sel => (sel.name.toTermName == sym.name || sel.name.toTypeName == sym.name) && altName.map(n => n.toTermName == sel.rename).getOrElse(true))
-        def dealiasedSelector =
+
+        val selectionsToDealias: List[SingleDenotation] =
+          val typeSelections = sels.flatMap(n => qualTpe.member(n.name.toTypeName).alternatives)
+          val termSelections = sels.flatMap(n => qualTpe.member(n.name.toTermName).alternatives)
+          typeSelections ::: termSelections
+
+        val qualHasSymbol: Boolean =
+          val simpleSelections = qualTpe.member(sym.name).alternatives
+          simpleSelections.exists(d => d.symbol == sym || d.symbol.dealias == dealiasedSym)
+            || selectionsToDealias.exists(d => d.symbol.dealias == dealiasedSym)
+
+        def selector: Option[ImportSelector] =
+          sels.find(sel => sym.name.toTermName == sel.name && altName.forall(n => n.toTermName == sel.rename))
+
+        def dealiasedSelector: Option[ImportSelector] =
           if isDerived then
-            sels.flatMap(sel => selectionsToDealias.map(m => (sel, m.symbol))).collect {
+            sels.flatMap(sel => selectionsToDealias.map(m => (sel, m.symbol))).collectFirst {
               case (sel, sym) if sym.dealias == dealiasedSym => sel
-            }.headOption
+            }
           else None
-        def givenSelector = if sym.is(Given) || sym.is(Implicit)
-          then sels.filter(sel => sel.isGiven && !sel.bound.isEmpty).find(sel => sel.boundTpe =:= sym.info)
+
+        def givenSelector: Option[ImportSelector] =
+          if sym.is(Given) || sym.is(Implicit) then
+            sels.filter(sel => sel.isGiven && !sel.bound.isEmpty).find(sel => sel.boundTpe =:= sym.info)
           else None
-        def wildcard = sels.find(sel => sel.isWildcard && ((sym.is(Given) == sel.isGiven && sel.bound.isEmpty) || sym.is(Implicit)))
-        if sym.exists && qualHasSymbol && (!isAccessible || sym.isRenamedSymbol(altName)) then
+
+        def wildcard: Option[ImportSelector] =
+          sels.find(sel => sel.isWildcard && ((sym.is(Given) == sel.isGiven && sel.bound.isEmpty) || sym.is(Implicit)))
+
+        if qualHasSymbol && (!isAccessible || altName.exists(_.toSimpleName != sym.name.toSimpleName)) then
           selector.orElse(dealiasedSelector).orElse(givenSelector).orElse(wildcard) // selector with name or wildcard (or given)
         else
           None
-
-      private def isRenamedSymbol(symNameInScope: Option[Name])(using Context) =
-        sym.name != nme.NO_NAME && symNameInScope.exists(_.toSimpleName != sym.name.toSimpleName)
+      end isInImport
 
       private def dealias(using Context): Symbol =
         if sym.isType && sym.asType.denot.isAliasType then

From 0c1f090ec78a1ab078250fcd473b2ec8e80de282 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= <sjrdoeraene@gmail.com>
Date: Fri, 3 May 2024 09:50:03 +0200
Subject: [PATCH 08/10] Fix some indentation.

---
 .../tools/dotc/transform/CheckUnused.scala    | 37 ++++++++++---------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
index ee80bdac5017..15a73f18f323 100644
--- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
+++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
@@ -827,23 +827,24 @@ object CheckUnused:
   end UnusedData
 
   private object UnusedData:
-      enum ScopeType:
-        case Local
-        case Template
-        case ReplWrapper
-        case Other
-
-      object ScopeType:
-        /** return the scope corresponding to the enclosing scope of the given tree */
-        def fromTree(tree: tpd.Tree)(using Context): ScopeType = tree match
-          case tree: tpd.Template => if tree.symbol.name.isReplWrapperName then ReplWrapper else Template
-          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: Set[UnusedSymbol])
-      object UnusedResult:
-        val Empty = UnusedResult(Set.empty)
+    enum ScopeType:
+      case Local
+      case Template
+      case ReplWrapper
+      case Other
+
+    object ScopeType:
+      /** return the scope corresponding to the enclosing scope of the given tree */
+      def fromTree(tree: tpd.Tree)(using Context): ScopeType = tree match
+        case tree: tpd.Template => if tree.symbol.name.isReplWrapperName then ReplWrapper else Template
+        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: Set[UnusedSymbol])
+    object UnusedResult:
+      val Empty = UnusedResult(Set.empty)
+  end UnusedData
 
 end CheckUnused

From 975df4a136910fe0451514ef0dc34aa29a60aef7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= <sjrdoeraene@gmail.com>
Date: Fri, 3 May 2024 12:06:27 +0200
Subject: [PATCH 09/10] Simplify the logic for checking unused imports.

Instead of dealing with entire `tpd.Import`s at the end of the
scope, we eagerly flatten them into individual `ImportSelector`s.
We store them along with some data, including a mutable flag for
whether a selector has been used.

This allows to dramatically simplify `isInImport`, as well as more
aggressively cache the resolution of selectors. We also get rid of
the `IdentityHashMap`.

The algorithm is still O(n*m) where n is the number of imports in
a scope, and m the number of references found in that scope. It is
not entirely clear to me whether the previous logic was already
O(n*m) or worse (it may have included an additional p factor for
the number of possible selections from a given qualifier).

Regardless, it is already quite a bit faster than before, thanks
to smaller constant factors.
---
 .../tools/dotc/transform/CheckUnused.scala    | 200 ++++++++++--------
 tests/warn/i15503i.scala                      |   4 +-
 2 files changed, 112 insertions(+), 92 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
index 15a73f18f323..84bf705905b1 100644
--- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
+++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
@@ -2,6 +2,7 @@ package dotty.tools.dotc.transform
 
 import scala.annotation.tailrec
 
+import dotty.tools.uncheckedNN
 import dotty.tools.dotc.ast.tpd
 import dotty.tools.dotc.core.Symbols.*
 import dotty.tools.dotc.ast.tpd.{Inlined, TreeTraverser}
@@ -109,8 +110,8 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
       ctx
 
   override def prepareForSelect(tree: tpd.Select)(using Context): Context =
-    val name = tree.removeAttachment(OriginalName).orElse(Some(tree.name))
-    unusedDataApply(_.registerUsed(tree.symbol, name))
+    val name = tree.removeAttachment(OriginalName)
+    unusedDataApply(_.registerUsed(tree.symbol, name, includeForImport = tree.qualifier.span.isSynthetic))
 
   override def prepareForBlock(tree: tpd.Block)(using Context): Context =
     pushInBlockTemplatePackageDef(tree)
@@ -128,7 +129,7 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
       if !tree.symbol.is(Module) then
         ud.registerDef(tree)
       if tree.name.startsWith("derived$") && tree.typeOpt != NoType then
-        ud.registerUsed(tree.typeOpt.typeSymbol, None, true)
+        ud.registerUsed(tree.typeOpt.typeSymbol, None, isDerived = true)
       ud.addIgnoredUsage(tree.symbol)
     }
 
@@ -359,7 +360,7 @@ object CheckUnused:
     var unusedAggregate: Option[UnusedResult] = None
 
     /* IMPORTS */
-    private val impInScope = MutStack(MutList[tpd.Import]())
+    private val impInScope = MutStack(MutList[ImportSelectorData]())
     /**
      * We store the symbol along with their accessibility without import.
      * Accessibility to their definition in outer context/scope
@@ -369,7 +370,7 @@ object CheckUnused:
     private val usedInScope = MutStack(MutSet[(Symbol,Boolean, Option[Name], Boolean)]())
     private val usedInPosition = MutMap.empty[Name, MutSet[Symbol]]
     /* unused import collected during traversal */
-    private val unusedImport = new java.util.IdentityHashMap[ImportSelector, Unit]
+    private val unusedImport = MutList.empty[ImportSelectorData]
 
     /* LOCAL DEF OR VAL / Private Def or Val / Pattern variables */
     private val localDefInScope = MutList.empty[tpd.MemberDef]
@@ -409,16 +410,17 @@ object CheckUnused:
      * The optional name will be used to target the right import
      * as the same element can be imported with different renaming
      */
-    def registerUsed(sym: Symbol, name: Option[Name], isDerived: Boolean = false)(using Context): Unit =
+    def registerUsed(sym: Symbol, name: Option[Name], includeForImport: Boolean = true, isDerived: Boolean = false)(using Context): Unit =
       if sym.exists && !isConstructorOfSynth(sym) && !doNotRegister(sym) then
         if sym.isConstructor then
-          registerUsed(sym.owner, None) // constructor are "implicitly" imported with the class
+          registerUsed(sym.owner, None, includeForImport) // constructor are "implicitly" imported with the class
         else
           val accessibleAsIdent = sym.isAccessibleAsIdent
           def addIfExists(sym: Symbol): Unit =
             if sym.exists then
               usedDef += sym
-              usedInScope.top += ((sym, accessibleAsIdent, name, isDerived))
+              if includeForImport then
+                usedInScope.top += ((sym, accessibleAsIdent, name, isDerived))
           addIfExists(sym)
           addIfExists(sym.companionModule)
           addIfExists(sym.companionClass)
@@ -439,12 +441,27 @@ object CheckUnused:
 
     /** Register an import */
     def registerImport(imp: tpd.Import)(using Context): Unit =
-      if !tpd.languageImport(imp.expr).nonEmpty && !imp.isGeneratedByEnum && !isTransparentAndInline(imp) then
-        impInScope.top += imp
-        if currScopeType.top != ScopeType.ReplWrapper then // #18383 Do not report top-level import's in the repl as unused
-          for s <- imp.selectors do
-            if !shouldSelectorBeReported(imp, s) && !isImportExclusion(s) && !isImportIgnored(imp, s) then
-              unusedImport.put(s, ())
+      if
+        !tpd.languageImport(imp.expr).nonEmpty
+          && !imp.isGeneratedByEnum
+          && !isTransparentAndInline(imp)
+          && currScopeType.top != ScopeType.ReplWrapper // #18383 Do not report top-level import's in the repl as unused
+      then
+        val qualTpe = imp.expr.tpe
+
+        // Put wildcard imports at the end, because they have lower priority within one Import
+        val reorderdSelectors =
+          val (wildcardSels, nonWildcardSels) = imp.selectors.partition(_.isWildcard)
+          nonWildcardSels ::: wildcardSels
+
+        val newDataInScope =
+          for sel <- reorderdSelectors yield
+            val data = new ImportSelectorData(qualTpe, sel)
+            if shouldSelectorBeReported(imp, sel) || isImportExclusion(sel) || isImportIgnored(imp, sel) then
+              // Immediately mark the selector as used
+              data.markUsed()
+            data
+        impInScope.top.prependAll(newDataInScope)
     end registerImport
 
     /** Register (or not) some `val` or `def` according to the context, scope and flags */
@@ -482,40 +499,27 @@ object CheckUnused:
      * - If there are imports in this scope check for unused ones
      */
     def popScope()(using Context): Unit =
-      // used symbol in this scope
-      val used = usedInScope.pop().toSet
-      // used imports in this scope
-      val imports = impInScope.pop()
-      val kept = used.filterNot { (sym, isAccessible, optName, isDerived) =>
-        // keep the symbol for outer scope, if it matches **no** import
-        // This is the first matching wildcard selector
-        var selWildCard: Option[ImportSelector] = None
-
-        val matchedExplicitImport = imports.exists { imp =>
-          sym.isInImport(imp, isAccessible, optName, isDerived) match
-            case None => false
-            case optSel@Some(sel) if sel.isWildcard =>
-              if selWildCard.isEmpty then selWildCard = optSel
-              // We keep wildcard symbol for the end as they have the least precedence
-              false
-            case Some(sel) =>
-              unusedImport.remove(sel)
-              true
+      currScopeType.pop()
+      val usedInfos = usedInScope.pop()
+      val selDatas = impInScope.pop()
+
+      for usedInfo <- usedInfos do
+        val (sym, isAccessible, optName, isDerived) = usedInfo
+        val usedData = selDatas.find { selData =>
+          sym.isInImport(selData, isAccessible, optName, isDerived)
         }
-        if !matchedExplicitImport && selWildCard.isDefined then
-          unusedImport.remove(selWildCard.get)
-          true // a matching import exists so the symbol won't be kept for outer scope
-        else
-          matchedExplicitImport
-      }
-
-      // if there's an outer scope
-      if usedInScope.nonEmpty then
-        // we keep the symbols not referencing an import in this scope
-        // as it can be the only reference to an outer import
-        usedInScope.top ++= kept
-      // retrieve previous scope type
-      currScopeType.pop
+        usedData match
+          case Some(data) =>
+            data.markUsed()
+          case None =>
+            // Propagate the symbol one level up
+            if usedInScope.nonEmpty then
+              usedInScope.top += usedInfo
+      end for // each in `used`
+
+      for selData <- selDatas do
+        if !selData.isUsed then
+          unusedImport += selData
     end popScope
 
     /**
@@ -534,9 +538,8 @@ object CheckUnused:
 
       val sortedImp =
         if ctx.settings.WunusedHas.imports || ctx.settings.WunusedHas.strictNoImplicitWarn then
-          import scala.jdk.CollectionConverters.*
-          unusedImport.keySet().nn.iterator().nn.asScala
-            .map(d => UnusedSymbol(d.srcPos, d.name, WarnTypes.Imports)).toList
+          unusedImport.toList
+            .map(d => UnusedSymbol(d.selector.srcPos, d.selector.name, WarnTypes.Imports))
         else
           Nil
       // Partition to extract unset local variables from usedLocalDefs
@@ -697,52 +700,40 @@ object CheckUnused:
         }
 
       /** Given an import and accessibility, return selector that matches import<->symbol */
-      private def isInImport(imp: tpd.Import, isAccessible: Boolean, altName: Option[Name], isDerived: Boolean)(using Context): Option[ImportSelector] =
+      private def isInImport(selData: ImportSelectorData, isAccessible: Boolean, altName: Option[Name], isDerived: Boolean)(using Context): Boolean =
         assert(sym.exists)
 
-        val tpd.Import(qual, sels) = imp
-        val qualTpe = qual.tpe
-        val dealiasedSym = sym.dealias
-
-        val selectionsToDealias: List[SingleDenotation] =
-          val typeSelections = sels.flatMap(n => qualTpe.member(n.name.toTypeName).alternatives)
-          val termSelections = sels.flatMap(n => qualTpe.member(n.name.toTermName).alternatives)
-          typeSelections ::: termSelections
-
-        val qualHasSymbol: Boolean =
-          val simpleSelections = qualTpe.member(sym.name).alternatives
-          simpleSelections.exists(d => d.symbol == sym || d.symbol.dealias == dealiasedSym)
-            || selectionsToDealias.exists(d => d.symbol.dealias == dealiasedSym)
+        val selector = selData.selector
 
-        def selector: Option[ImportSelector] =
-          sels.find(sel => sym.name.toTermName == sel.name && altName.forall(n => n.toTermName == sel.rename))
-
-        def dealiasedSelector: Option[ImportSelector] =
-          if isDerived then
-            sels.flatMap(sel => selectionsToDealias.map(m => (sel, m.symbol))).collectFirst {
-              case (sel, sym) if sym.dealias == dealiasedSym => sel
-            }
-          else None
-
-        def givenSelector: Option[ImportSelector] =
-          if sym.is(Given) || sym.is(Implicit) then
-            sels.filter(sel => sel.isGiven && !sel.bound.isEmpty).find(sel => sel.boundTpe =:= sym.info)
-          else None
-
-        def wildcard: Option[ImportSelector] =
-          sels.find(sel => sel.isWildcard && ((sym.is(Given) == sel.isGiven && sel.bound.isEmpty) || sym.is(Implicit)))
-
-        if qualHasSymbol && (!isAccessible || altName.exists(_.toSimpleName != sym.name.toSimpleName)) then
-          selector.orElse(dealiasedSelector).orElse(givenSelector).orElse(wildcard) // selector with name or wildcard (or given)
+        if isAccessible && !altName.exists(_.toTermName != sym.name.toTermName) then
+          // Even if this import matches, it is pointless because the symbol would be accessible anyway
+          false
+        else if !selector.isWildcard then
+          if altName.exists(explicitName => selector.rename != explicitName.toTermName) then
+            // if there is an explicit name, it must match
+            false
+          else
+            if isDerived then
+              // See i15503i.scala, grep for "package foo.test.i17156"
+              selData.allSymbolsDealiasedForNamed.contains(dealias(sym))
+            else
+              selData.allSymbolsForNamed.contains(sym)
         else
-          None
+          // Wildcard
+          if !selData.qualTpe.member(sym.name).hasAltWith(_.symbol == sym) then
+            // The qualifier does not have the target symbol as a member
+            false
+          else
+            if selector.isGiven then
+              // Further check that the symbol is a given or implicit and conforms to the bound
+              sym.isOneOf(Given | Implicit)
+                && (selector.bound.isEmpty || sym.info <:< selector.boundTpe)
+            else
+              // Normal wildcard, check that the symbol is not a given (but can be implicit)
+              !sym.is(Given)
+        end if
       end isInImport
 
-      private def dealias(using Context): Symbol =
-        if sym.isType && sym.asType.denot.isAliasType then
-          sym.asType.typeRef.dealias.typeSymbol
-        else sym
-
       /** Annotated with @unused */
       private def isUnusedAnnot(using Context): Boolean =
         sym.annotations.exists(a => a.symbol == ctx.definitions.UnusedAnnot)
@@ -840,6 +831,29 @@ object CheckUnused:
         case _:tpd.Block => Local
         case _ => Other
 
+    final class ImportSelectorData(val qualTpe: Type, val selector: ImportSelector):
+      private var myUsed: Boolean = false
+
+      def markUsed(): Unit = myUsed = true
+
+      def isUsed: Boolean = myUsed
+
+      private var myAllSymbols: Set[Symbol] | Null = null
+
+      def allSymbolsForNamed(using Context): Set[Symbol] =
+        if myAllSymbols == null then
+          val allDenots = qualTpe.member(selector.name).alternatives ::: qualTpe.member(selector.name.toTypeName).alternatives
+          myAllSymbols = allDenots.map(_.symbol).toSet
+        myAllSymbols.uncheckedNN
+
+      private var myAllSymbolsDealiased: Set[Symbol] | Null = null
+
+      def allSymbolsDealiasedForNamed(using Context): Set[Symbol] =
+        if myAllSymbolsDealiased == null then
+          myAllSymbolsDealiased = allSymbolsForNamed.map(sym => dealias(sym))
+        myAllSymbolsDealiased.uncheckedNN
+    end ImportSelectorData
+
     case class UnusedSymbol(pos: SrcPos, name: Name, warnType: WarnTypes)
     /** A container for the results of the used elements analysis */
     case class UnusedResult(warnings: Set[UnusedSymbol])
@@ -847,4 +861,10 @@ object CheckUnused:
       val Empty = UnusedResult(Set.empty)
   end UnusedData
 
+  private def dealias(symbol: Symbol)(using Context): Symbol =
+    if symbol.isType && symbol.asType.denot.isAliasType then
+      symbol.asType.typeRef.dealias.typeSymbol
+    else
+      symbol
+
 end CheckUnused
diff --git a/tests/warn/i15503i.scala b/tests/warn/i15503i.scala
index 329b81327288..8a8ed487477a 100644
--- a/tests/warn/i15503i.scala
+++ b/tests/warn/i15503i.scala
@@ -270,7 +270,7 @@ package foo.test.i16679b:
     given x: myPackage.CaseClassName[secondPackage.CoolClass] = null
 
   object secondPackage:
-    import myPackage.CaseClassName // OK
+    import myPackage.CaseClassName // warn
     import Foo.x
     case class CoolClass(i: Int)
     println(summon[myPackage.CaseClassName[CoolClass]])
@@ -312,4 +312,4 @@ package foo.test.i17117:
 
       val test = t1.test
     }
-  }
\ No newline at end of file
+  }

From 8553bfcc2dc17073b49b3fbed82fa1b7079abcc7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= <sjrdoeraene@gmail.com>
Date: Fri, 3 May 2024 17:40:12 +0200
Subject: [PATCH 10/10] Move the isAccessible test up to registerSym, instead
 of isInImport.

That test does not rely on any information dependent on the import
selectors. It only relies on information at the use site. Eagerly
checking it means we do not put as many symbols into the
`usedInScope` set, which is good because it is one of the
complexity factors of the unused-imports analysis.
---
 .../tools/dotc/transform/CheckUnused.scala    | 23 ++++++++++---------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
index 84bf705905b1..bd4ef73d6eea 100644
--- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
+++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
@@ -367,7 +367,7 @@ object CheckUnused:
      *
      * See the `isAccessibleAsIdent` extension method below in the file
      */
-    private val usedInScope = MutStack(MutSet[(Symbol,Boolean, Option[Name], Boolean)]())
+    private val usedInScope = MutStack(MutSet[(Symbol, Option[Name], Boolean)]())
     private val usedInPosition = MutMap.empty[Name, MutSet[Symbol]]
     /* unused import collected during traversal */
     private val unusedImport = MutList.empty[ImportSelectorData]
@@ -415,12 +415,16 @@ object CheckUnused:
         if sym.isConstructor then
           registerUsed(sym.owner, None, includeForImport) // constructor are "implicitly" imported with the class
         else
-          val accessibleAsIdent = sym.isAccessibleAsIdent
+          // If the symbol is accessible in this scope without an import, do not register it for unused import analysis
+          val includeForImport1 =
+            includeForImport
+              && (name.exists(_.toTermName != sym.name.toTermName) || !sym.isAccessibleAsIdent)
+
           def addIfExists(sym: Symbol): Unit =
             if sym.exists then
               usedDef += sym
-              if includeForImport then
-                usedInScope.top += ((sym, accessibleAsIdent, name, isDerived))
+              if includeForImport1 then
+                usedInScope.top += ((sym, name, isDerived))
           addIfExists(sym)
           addIfExists(sym.companionModule)
           addIfExists(sym.companionClass)
@@ -504,9 +508,9 @@ object CheckUnused:
       val selDatas = impInScope.pop()
 
       for usedInfo <- usedInfos do
-        val (sym, isAccessible, optName, isDerived) = usedInfo
+        val (sym, optName, isDerived) = usedInfo
         val usedData = selDatas.find { selData =>
-          sym.isInImport(selData, isAccessible, optName, isDerived)
+          sym.isInImport(selData, optName, isDerived)
         }
         usedData match
           case Some(data) =>
@@ -700,15 +704,12 @@ object CheckUnused:
         }
 
       /** Given an import and accessibility, return selector that matches import<->symbol */
-      private def isInImport(selData: ImportSelectorData, isAccessible: Boolean, altName: Option[Name], isDerived: Boolean)(using Context): Boolean =
+      private def isInImport(selData: ImportSelectorData, altName: Option[Name], isDerived: Boolean)(using Context): Boolean =
         assert(sym.exists)
 
         val selector = selData.selector
 
-        if isAccessible && !altName.exists(_.toTermName != sym.name.toTermName) then
-          // Even if this import matches, it is pointless because the symbol would be accessible anyway
-          false
-        else if !selector.isWildcard then
+        if !selector.isWildcard then
           if altName.exists(explicitName => selector.rename != explicitName.toTermName) then
             // if there is an explicit name, it must match
             false