Merge pull request #308 from scala/backport-lts-3.3-22751

Backport "Restore resolving prefixes of implicit Ident" to 3.3 LTS

Author: tgodzik

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

@@ -58,6 +58,12 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
         || tree.srcPos.isZeroExtentSynthetic
         || refInfos.inlined.exists(_.sourcePos.contains(tree.srcPos.sourcePos))
       if resolving && !ignoreTree(tree) then
+        def loopOverPrefixes(prefix: Type, depth: Int): Unit =
+          if depth < 10 && prefix.exists && !prefix.classSymbol.isEffectiveRoot then
+            resolveUsage(prefix.classSymbol, nme.NO_NAME, NoPrefix)
+            loopOverPrefixes(prefix.normalizedPrefix, depth + 1)
+        if tree.srcPos.isZeroExtentSynthetic then
+          loopOverPrefixes(tree.typeOpt.normalizedPrefix, depth = 0)
         resolveUsage(tree.symbol, tree.name, tree.typeOpt.importPrefix.skipPackageObject)
     else if tree.hasType then
       resolveUsage(tree.tpe.classSymbol, tree.name, tree.tpe.importPrefix.skipPackageObject)
@@ -116,7 +122,7 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
     tree
 
   override def prepareForMatch(tree: Match)(using Context): Context =
-    // exonerate case.pat against tree.selector (simple var pat only for now)
+    // allow case.pat against tree.selector (simple var pat only for now)
     tree.selector match
     case Ident(nm) => tree.cases.foreach(k => allowVariableBindings(List(nm), List(k.pat)))
     case _ =>
@@ -138,9 +144,9 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
     refInfos.inlined.push(tree.call.srcPos)
     ctx
   override def transformInlined(tree: Inlined)(using Context): tree.type =
+    transformAllDeep(tree.expansion) // traverse expansion with nonempty inlined stack to avoid registering defs
     val _ = refInfos.inlined.pop()
-    if !tree.call.isEmpty && phaseMode.eq(PhaseMode.Aggregate) then
-      transformAllDeep(tree.call)
+    transformAllDeep(tree.call)
     tree
 
   override def prepareForBind(tree: Bind)(using Context): Context =
@@ -158,11 +164,7 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
     traverseAnnotations(tree.symbol)
     if tree.name.startsWith("derived$") && tree.hasType then
       def loop(t: Tree): Unit = t match
-        case Ident(name)  =>
-          val target =
-            val ts0 = t.tpe.typeSymbol
-            if ts0.is(ModuleClass) then ts0.companionModule else ts0
-          resolveUsage(target, name, t.tpe.underlyingPrefix.skipPackageObject)
+        case Ident(name)  => resolveUsage(t.tpe.typeSymbol, name, t.tpe.underlyingPrefix.skipPackageObject)
         case Select(t, _) => loop(t)
         case _            =>
       tree.getAttachment(OriginalTypeClass).foreach(loop)
@@ -272,8 +274,9 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
    *  For Select, lint does not look up `<empty>.scala` (so top-level syms look like magic) but records `scala.Int`.
    *  For Ident, look-up finds the root import as usual. A competing import is OK because higher precedence.
    */
-  def resolveUsage(sym: Symbol, name: Name, prefix: Type)(using Context): Unit =
+  def resolveUsage(sym0: Symbol, name: Name, prefix: Type)(using Context): Unit =
     import PrecedenceLevels.*
+    val sym = sym0.userSymbol
 
     def matchingSelector(info: ImportInfo): ImportSelector | Null =
       val qtpe = info.site
@@ -319,7 +322,7 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
 
     // 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 || defn.topClasses(sym.owner) then return
+    if !sym.exists || sym.isPrimaryConstructor || sym.isEffectiveRoot || defn.topClasses(sym.owner) then return
 
     // Find the innermost, highest precedence. Contexts have no nesting levels but assume correctness.
     // If the sym is an enclosing definition (the owner of a context), it does not count toward usages.
@@ -454,11 +457,15 @@ object CheckUnused:
         if !tree.name.isInstanceOf[DerivedName] then
           pats.addOne((tree.symbol, tree.namePos))
       case tree: NamedDefTree =>
-        if (tree.symbol ne NoSymbol) && !tree.name.isWildcard && !tree.hasAttachment(NoWarn) then
-          defs.addOne((tree.symbol, tree.namePos))
+        if (tree.symbol ne NoSymbol)
+          && !tree.name.isWildcard
+          && !tree.hasAttachment(NoWarn)
+          && !tree.symbol.is(ModuleVal) // track only the ModuleClass using the object symbol, with correct namePos
+        then
+          defs.addOne((tree.symbol.userSymbol, tree.namePos))
       case _ =>
         if tree.symbol ne NoSymbol then
-          defs.addOne((tree.symbol, tree.srcPos))
+          defs.addOne((tree.symbol, tree.srcPos)) // TODO is this a code path
 
     val inlined = Stack.empty[SrcPos] // enclosing call.srcPos of inlined code (expansions)
     var inliners = 0 // depth of inline def (not inlined yet)
@@ -518,7 +525,7 @@ object CheckUnused:
     def checkPrivate(sym: Symbol, pos: SrcPos) =
       if ctx.settings.WunusedHas.privates
         && !sym.isPrimaryConstructor
-        && sym.is(Private, butNot = SelfName | Synthetic | CaseAccessor)
+        && !sym.isOneOf(SelfName | Synthetic | CaseAccessor)
         && !sym.name.is(BodyRetainerName)
         && !sym.isSerializationSupport
         && !(sym.is(Mutable) && sym.isSetter && sym.owner.is(Trait)) // tracks sym.underlyingSymbol sibling getter
@@ -763,7 +770,7 @@ object CheckUnused:
     for (sym, pos) <- infos.defs.iterator if !sym.hasAnnotation(defn.UnusedAnnot) do
       if infos.refs(sym) then
         checkUnassigned(sym, pos)
-      else if sym.is(Private, butNot = ParamAccessor) then
+      else if sym.isEffectivelyPrivate then
         checkPrivate(sym, pos)
       else if sym.is(Param, butNot = Given | Implicit) then
         checkParam(sym, pos)
@@ -903,6 +910,13 @@ object CheckUnused:
         val base = if owner.classInfo.selfInfo != NoType then owner.thisType else owner.info
         base.baseClasses.drop(1).iterator.exists(sym.overriddenSymbol(_).exists)
       }
+    def isEffectivelyPrivate: Boolean =
+      sym.is(Private, butNot = ParamAccessor)
+      || sym.owner.isAnonymousClass && !sym.nextOverriddenSymbol.exists
+    // pick the symbol the user wrote for purposes of tracking
+    inline def userSymbol: Symbol=
+      if sym.denot.is(ModuleClass) then sym.denot.companionModule else sym
+
   extension (sel: ImportSelector)
     def boundTpe: Type = sel.bound match
       case untpd.TypedSplice(tree) => tree.tpe

tests/warn/i22681.scala

@@ -8,7 +8,7 @@ class C:
   def f: Runnable { def u: Int } = new Runnable with T:
     private def v = 42 // avoid g judged too trivial to warn
     def run() = ()
-    def g = v // LTS specific: no warn, althougheffectively private member is unused
+    def g = v // warn, effectively private member is unused
     def t = v // nowarn
     def u = v // nowarn because leaked by refinement
   val v: Runnable { def u: Int } = new Runnable:

tests/warn/i22744.scala

@@ -0,0 +1,35 @@
+
+//> using options -Wunused:privates -Werror
+
+object test {
+  private trait Foo[A] { val value: A }
+
+  private object Foo { // no warn prefix of implicit value
+    given int: Foo[Int] = new Foo[Int] { val value = 1 }
+  }
+
+  val i = summon[Foo[Int]].value
+}
+
+object supplement {
+  private trait Foo[A] { val value: A }
+
+  private object Foo { // no warn prefix of implicit value
+    given int: Foo[Int] = new Foo[Int] { val value = 1 }
+  }
+
+  private def fooValue[A](using f: Foo[A]): A = f.value
+
+  val i = fooValue[Int]
+}
+
+package p:
+  private trait Foo[A] { val value: A }
+
+  private object Foo { // no warn prefix of implicit value
+    given int: Foo[Int] = new Foo[Int] { val value = 1 }
+  }
+
+  private def fooValue[A](using f: Foo[A]): A = f.value
+
+  val i = fooValue[Int]