Tweaks per review

compiler/src/dotty/tools/dotc/core/Definitions.scala

@@ -836,6 +836,7 @@ class Definitions {
   @tu lazy val QuotedExprClass: ClassSymbol = requiredClass("scala.quoted.Expr")
 
   @tu lazy val QuotesClass: ClassSymbol = requiredClass("scala.quoted.Quotes")
+    @tu lazy val Quotes_reflectModule: Symbol = QuotesClass.requiredClass("reflectModule")
     @tu lazy val Quotes_reflect: Symbol = QuotesClass.requiredValue("reflect")
       @tu lazy val Quotes_reflect_asTerm: Symbol = Quotes_reflect.requiredMethod("asTerm")
       @tu lazy val Quotes_reflect_Apply: Symbol = Quotes_reflect.requiredValue("Apply")
@@ -957,6 +958,7 @@ class Definitions {
   def NonEmptyTupleClass(using Context): ClassSymbol = NonEmptyTupleTypeRef.symbol.asClass
     lazy val NonEmptyTuple_tail: Symbol = NonEmptyTupleClass.requiredMethod("tail")
   @tu lazy val PairClass: ClassSymbol = requiredClass("scala.*:")
+    @tu lazy val PairClass_unapply: Symbol = PairClass.companionModule.requiredMethod("unapply")
 
   @tu lazy val TupleXXLClass: ClassSymbol = requiredClass("scala.runtime.TupleXXL")
   def TupleXXLModule(using Context): Symbol = TupleXXLClass.companionModule

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

@@ -52,7 +52,7 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
 
   override def transformIdent(tree: Ident)(using Context): tree.type =
     if tree.symbol.exists then
-      // resolve if inlined at the position of the call, or is zero extent summon
+      // if in an inline expansion, resolve at summonInline (synthetic pos) or in an enclosing call site
       val resolving =
            refInfos.inlined.isEmpty
         || tree.srcPos.isZeroExtentSynthetic
@@ -111,7 +111,7 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
   override def prepareForMatch(tree: Match)(using Context): Context =
     // exonerate case.pat against tree.selector (simple var pat only for now)
     tree.selector match
-    case Ident(nm) => tree.cases.foreach(k => absolveVariableBindings(List(nm), List(k.pat)))
+    case Ident(nm) => tree.cases.foreach(k => allowVariableBindings(List(nm), List(k.pat)))
     case _ =>
     ctx
   override def transformMatch(tree: Match)(using Context): tree.type =
@@ -244,24 +244,27 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
       cases.foreach(transformAllDeep)
     case ByNameTypeTree(result) =>
       transformAllDeep(result)
-    case _: InferredTypeTree => // do nothing
-    case _: Export => // nothing to do
-    case _ if tree.isType =>
-      //println(s"OTHER TYPE ${tree.getClass} ${tree.show}")
+    //case _: InferredTypeTree => // do nothing
+    //case _: Export => // nothing to do
+    //case _ if tree.isType =>
     case _ =>
-      //println(s"OTHER ${tree.getClass} ${tree.show}")
     tree
 
   private def traverseAnnotations(sym: Symbol)(using Context): Unit =
     for annot <- sym.denot.annotations do
       transformAllDeep(annot.tree)
 
+  // if sym is not an enclosing element, record the reference
+  def refUsage(sym: Symbol)(using Context): Unit =
+    if !ctx.outersIterator.exists(cur => cur.owner eq sym) then
+      refInfos.refs.addOne(sym)
+
   /** Look up a reference in enclosing contexts to determine whether it was introduced by a definition or import.
    *  The binding of highest precedence must be correct.
    */
   def resolveUsage(sym: Symbol, name: Name, prefix: Type)(using Context): Unit =
     def matchingSelector(info: ImportInfo): ImportSelector | Null =
-      val qtpe = info.site //info.qualifier.tpe.nn
+      val qtpe = info.site
       def loop(sels: List[ImportSelector]): ImportSelector | Null =
         sels match
         case sel :: sels if sel.isWildcard =>
@@ -303,33 +306,40 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
     if !sym.exists then return
 
     // Names are resolved by definitions and imports, which have four precedence levels:
-    // 1. def from this compilation unit
-    // 2. specific import
-    // 3. wildcard import
-    // 4. def from another compilation unit via enclosing package
-    // We find the innermost, highest precedence. We have no nesting levels but assume correctness.
+    object PrecedenceLevels:
+      opaque type Precedence = Int
+      inline def NoPrecedence: Precedence = 5
+      inline def OtherUnit: Precedence = 4   // root import or def from another compilation unit via enclosing package
+      inline def Wildcard: Precedence = 3    // wildcard import
+      inline def NamedImport: Precedence = 2 // specific import
+      inline def Definition: Precedence = 1  // def from this compilation unit
+      extension (p: Precedence) inline def weakerThan(q: Precedence) = p > q
+    import PrecedenceLevels.*
+
+    // 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.
     val isLocal = sym.isLocalToBlock
-    var foundEnclosing = false
     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 = Int.MaxValue // of current resolution; lower is higher
+    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()
       if cur.owner eq sym then
-        foundEnclosing = true
-        done = true
+        return // found enclosing definition
       else if isLocal then
-        if cur.owner eq sym.owner then done = true // only checking for enclosing
+        if cur.owner eq sym.owner then
+          done = true // for local def, just checking that it is not enclosing
       else
         cached =
           cur.property(resolvedKey) match
           case Some(res) =>
-            if precedence > 4 then cachePoint = cur // conservative, cache must be nested below the result context
+            // conservative, cache must be nested below the result context
+            if precedence == NoPrecedence then
+              cachePoint = cur
             res.saw(sym, name, prefix)
           case _ => false
         if cached then
@@ -339,44 +349,40 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
           val sel = matchingSelector(cur.importInfo.nn)
           if sel != null then
             if cur.importInfo.nn.isRootImport then
-              if precedence > 4 then
-                precedence = 4
+              if precedence.weakerThan(OtherUnit) then
+                precedence = OtherUnit
                 candidate = cur
                 importer = sel
               done = true
             else if sel.isWildcard then
-              if precedence > 3 then
-                precedence = 3
+              if precedence.weakerThan(Wildcard) then
+                precedence = Wildcard
                 candidate = cur
                 importer = sel
             else
-              if precedence > 2 then
-                precedence = 2
+              if precedence.weakerThan(NamedImport) then
+                precedence = NamedImport
                 candidate = cur
                 importer = sel
         else if checkMember(cur.owner) then
           if sym.srcPos.sourcePos.source == ctx.source then
-            precedence = 1
+            precedence = Definition
             candidate = cur
             importer = null
             done = true
-          else if precedence > 4 then
-            precedence = 4
+          else if precedence.weakerThan(OtherUnit) then
+            precedence = OtherUnit
             candidate = cur
     end while
-    if !foundEnclosing then
-      refInfos.refs.addOne(sym)
-      if candidate != NoContext && candidate.isImportContext && importer != null
-      then refInfos.sels.put(importer, ())
-      if !cached then
-        addCached(cachePoint)
-      if cachePoint ne ctx then
-        addCached(ctx)
+    // record usage and possibly an import
+    refInfos.refs.addOne(sym)
+    if candidate != NoContext && candidate.isImportContext && importer != null then
+      refInfos.sels.put(importer, ())
+    if !cached then
+      addCached(cachePoint)
+    if cachePoint ne ctx then
+      addCached(ctx)
   end resolveUsage
-  // if sym is not an enclosing element, record the reference
-  def refUsage(sym: Symbol)(using Context): Unit =
-    if !ctx.outersIterator.exists(cur => cur.owner eq sym) then
-      refInfos.refs.addOne(sym)
 end CheckUnused
 
 object CheckUnused:
@@ -399,7 +405,7 @@ object CheckUnused:
   /** Attachment holding the name of an Ident as written by the user. */
   val OriginalName = Property.StickyKey[Name]
 
-  /** Suppress warning in a tree, such as a patvar absolved of unused warning by special naming convention. */
+  /** Suppress warning in a tree, such as a patvar name allowed by special convention. */
   val NoWarn = Property.StickyKey[Unit]
 
   /** Ignore reference. */
@@ -436,11 +442,12 @@ object CheckUnused:
         if (tree.symbol ne NoSymbol) && !tree.name.isWildcard then
           defs.addOne((tree.symbol, tree.namePos))
       case _ =>
+        //println(s"OTHER ${tree.symbol}")
         if tree.symbol ne NoSymbol then
           defs.addOne((tree.symbol, tree.srcPos))
 
-    val inlined = Stack.empty[SrcPos] // enclosing call.srcPos of inlined code
-    var inliners = 0 // depth of inline def
+    val inlined = Stack.empty[SrcPos] // enclosing call.srcPos of inlined code (expansions)
+    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)
@@ -479,7 +486,7 @@ object CheckUnused:
           warnAt(pos)(UnusedSymbol.privateMembers)
       else if sym.is(Param, butNot = Given | Implicit) then
         val m = sym.owner
-        def forgiven =
+        def allowed =
           val dd = defn
              m.isDeprecated
           || m.is(Synthetic) && !m.isAnonymousFunction
@@ -515,12 +522,12 @@ object CheckUnused:
             warnAt(pos)(UnusedSymbol.explicitParams)
         end checkExplicit
         if !infos.skip(m)
-          && !forgiven
+          && !allowed
         then
           checkExplicit()
       else if sym.is(Param) then // Given | Implicit
         val m = sym.owner
-        def forgiven =
+        def allowed =
           val dd = defn
              m.isDeprecated
           || m.is(Synthetic)
@@ -536,7 +543,7 @@ object CheckUnused:
           || sym.info.isInstanceOf[RefinedType] // can't be expressed as a context bound
         if ctx.settings.WunusedHas.implicits
           && !infos.skip(m)
-          && !forgiven
+          && !allowed
         then
           if m.isPrimaryConstructor then
             val alias = m.owner.info.member(sym.name)
@@ -710,75 +717,60 @@ object CheckUnused:
   // The RHS of a def is too trivial to warn about unused params, e.g. def f(x: Int) = ???
   def isUnconsuming(rhs: Tree)(using Context): Boolean =
         rhs.symbol == defn.Predef_undefined
-     || rhs.tpe =:= defn.NothingType
-     || rhs.isInstanceOf[Literal]
+     || rhs.tpe =:= defn.NothingType // compiletime.error
+     || rhs.isInstanceOf[Literal] // 42
      || rhs.tpe.match
         case ConstantType(_) => true
         case tp: TermRef => tp.underlying.classSymbol.is(Module) // Scala 2 SingleType
         case _ => false
      //|| isPurePath(rhs) // a bit strong
      || rhs.match
         case Block((dd @ DefDef(anonfun, paramss, _, _)) :: Nil, Closure(Nil, Ident(nm), _)) =>
-             isAnonymousFunctionName(anonfun)
-          && anonfun == nm
+             anonfun == nm // isAnonymousFunctionName(anonfun)
           && paramss.match
              case (ValDef(contextual, _, _) :: Nil) :: Nil =>
                   contextual.is(ContextFunctionParamName)
-               && isUnconsuming(dd.rhs)
+               && isUnconsuming(dd.rhs) // rhs was wrapped in a context function
              case _ => false
-        case Block(Nil, Literal(u)) => u.tpe =:= defn.UnitType
+        case Block(Nil, Literal(u)) => u.tpe =:= defn.UnitType // def f(x: X) = {}
         case This(_) => true
         case Ident(_) => rhs.symbol.is(ParamAccessor)
         case Typed(rhs, _) => isUnconsuming(rhs)
         case _ => false
 
-  def absolveVariableBindings(ok: List[Name], args: List[Tree]): Unit =
-    ok.zip(args).foreach: (param, arg) =>
-      arg match
-      case Bind(`param`, _) => arg.withAttachment(NoWarn, ())
+  def allowVariableBindings(ok: List[Name], args: List[Tree]): Unit =
+    ok.zip(args).foreach:
+      case (param, arg @ Bind(p, _)) if param == p => arg.withAttachment(NoWarn, ())
       case _ =>
 
   // NoWarn Binds if the name matches a "canonical" name, e.g. case element name
   val nowarner = new TreeTraverser:
     def traverse(tree: Tree)(using Context) = tree match
       case UnApply(fun, _, args) =>
-        def untuple = defn.PairClass.companionModule.requiredMethod("unapply")
         val unapplied = tree.tpe.finalResultType.dealias.typeSymbol
         if unapplied.is(CaseClass) then
-          absolveVariableBindings(unapplied.primaryConstructor.info.firstParamNames, args)
-        else if fun.symbol == untuple then
+          allowVariableBindings(unapplied.primaryConstructor.info.firstParamNames, args)
+        else if fun.symbol == defn.PairClass_unapply then
           val ok = fun.symbol.info match
             case PolyType(tycon, MethodTpe(_, _, AppliedType(_, tprefs))) =>
               tprefs.collect:
                 case ref: TypeParamRef => termName(ref.binder.paramNames(ref.paramNum).toString.toLowerCase.nn)
             case _ => Nil
-          absolveVariableBindings(ok, args)
+          allowVariableBindings(ok, args)
         else if fun.symbol == defn.TypeTest_unapply then
           () // just recurse into args
-          /*
-          fun match
-          case Select(qual, nme.unapply) =>
-            qual.tpe.underlying.finalResultType match
-            case AppliedType(tycon, args) if tycon.typeSymbol == defn.TypeTestClass =>
-              val target = args(1).dealias.typeSymbol
-              if target.is(CaseClass) then
-                absolveVariableBindings(target.primaryConstructor.info.firstParamNames, args)
-            case _ =>
-          case _ =>
-          */
         else
-          val Quotes_reflect: Symbol = defn.QuotesClass.requiredClass("reflectModule")
-          if unapplied.exists && unapplied.owner == Quotes_reflect then
+          if unapplied.exists && unapplied.owner == defn.Quotes_reflectModule then
             // cheapy search for parameter names via java reflection of Trees
             // in lieu of drilling into requiredClass("scala.quoted.runtime.impl.QuotesImpl")
             // ...member("reflect")...member(unapplied.name.toTypeName)
             // with aliases into requiredModule("dotty.tools.dotc.ast.tpd")
             val implName = s"dotty.tools.dotc.ast.Trees$$${unapplied.name}"
             try
               import scala.language.unsafeNulls
-              val clz = Class.forName(implName)
+              val clz = Class.forName(implName) // TODO improve to use class path or reflect
               val ok = clz.getConstructors.head.getParameters.map(p => termName(p.getName)).toList.init
-              absolveVariableBindings(ok, args)
+              allowVariableBindings(ok, args)
             catch case _: ClassNotFoundException => ()
         args.foreach(traverse)
       case tree => traverseChildren(tree)
@@ -830,7 +822,7 @@ object CheckUnused:
       case _ => false
 
   extension (imp: Import)
-    /** Is it the first import clause in a statement? `a.x` in `import a.x, b,{y, z}` */
+    /** Is it the first import clause in a statement? `a.x` in `import a.x, b.{y, z}` */
     def isPrimaryClause(using Context): Boolean =
       val span = imp.srcPos.span
       span.start != span.point // primary clause starts at `import` keyword
@@ -844,15 +836,15 @@ object CheckUnused:
      *  specifically does import an implicit.
      *  Similarly, import of CanEqual must not warn, as it is always witness.
      */
-    def isLoose(sel: ImportSelector)(using Context): Boolean = ctx.settings.WunusedHas.strictNoImplicitWarn && (
-         sel.isWildcard
-      || imp.expr.tpe.member(sel.name.toTermName).hasAltWith(_.symbol.isOneOf(GivenOrImplicit))
-      || imp.expr.tpe.member(sel.name.toTypeName).hasAltWith(_.symbol.isOneOf(GivenOrImplicit))
-    ) || (
-         sel.isWildcard && sel.isGiven
-      && imp.expr.tpe.allMembers.exists(_.symbol.isCanEqual)
-      || imp.expr.tpe.member(sel.name.toTermName).hasAltWith(_.symbol.isCanEqual)
-    )
+    def isLoose(sel: ImportSelector)(using Context): Boolean =
+      if ctx.settings.WunusedHas.strictNoImplicitWarn then
+        if sel.isWildcard
+          || imp.expr.tpe.member(sel.name.toTermName).hasAltWith(_.symbol.isOneOf(GivenOrImplicit))
+          || imp.expr.tpe.member(sel.name.toTypeName).hasAltWith(_.symbol.isOneOf(GivenOrImplicit))
+        then return true
+      if sel.isWildcard && sel.isGiven
+      then imp.expr.tpe.allMembers.exists(_.symbol.isCanEqual)
+      else imp.expr.tpe.member(sel.name.toTermName).hasAltWith(_.symbol.isCanEqual)
 
   extension (pos: SrcPos)
     def isZeroExtentSynthetic: Boolean = pos.span.isSynthetic && pos.span.start == pos.span.end

tests/warn/i15503f.scala

@@ -32,11 +32,14 @@ object Example:
       case '{ None } => Some(None)
       case '{ ${Expr(opt)} : Some[T] } => Some(opt)
       case _ => None
-  given OptionFromExprNoisy[T](using Type[T], FromExpr[T]): FromExpr[Option[T]] with
+
+object ExampleWithoutWith:
+  import scala.quoted.*
+  given [T] => (Type[T], FromExpr[T]) => FromExpr[Option[T]]:
     def unapply(x: Expr[Option[T]])(using Quotes) = x match
       case '{ Option[T](${Expr(y)}) } => Some(Option(y))
       case '{ None } => Some(None)
-      //case '{ ${Expr(opt)} : Some[T] } => Some(opt) // make Type param unused after typer
+      case '{ ${Expr(opt)} : Some[T] } => Some(opt)
       case _ => None
 
 //absolving names on matches of quote trees requires consulting non-abstract types in QuotesImpl

tests/warn/i15503kb/power.scala

@@ -1,6 +1,5 @@
 
 object Power:
-  import scala.concurrent.* // warn [taps mic]
   import scala.math.pow as power
   import scala.quoted.*
   inline def powerMacro(x: Double, inline n: Int): Double = ${ powerCode('x, 'n) }

tests/warn/i15503kb/square.scala

@@ -1,6 +1,5 @@
-//> using options -Wunused:all
+//> using options -Werror -Wunused:all
 
 object PowerUser:
-  import scala.concurrent.* // warn [taps mic]
   import Power.*
   def square(x: Double): Double = powerMacro(x, 2)