Simplify handling of Assign in unused check

Author: som-snytt

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

@@ -51,7 +51,6 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
     tree
 
   override def transformIdent(tree: Ident)(using Context): tree.type =
-    refInfos.isAssignment = tree.hasAttachment(AssignmentTarget)
     if tree.symbol.exists then
       // if in an inline expansion, resolve at summonInline (synthetic pos) or in an enclosing call site
       val resolving =
@@ -67,12 +66,10 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
         resolveUsage(tree.symbol, tree.name, tree.typeOpt.importPrefix.skipPackageObject, imports = resolving)
     else if tree.hasType then
       resolveUsage(tree.tpe.classSymbol, tree.name, tree.tpe.importPrefix.skipPackageObject)
-    refInfos.isAssignment = false
     tree
 
   // import x.y; y may be rewritten x.y, also import x.z as y
   override def transformSelect(tree: Select)(using Context): tree.type =
-    refInfos.isAssignment = tree.hasAttachment(AssignmentTarget)
     val name = tree.removeAttachment(OriginalName).getOrElse(nme.NO_NAME)
     inline def isImportable = tree.qualifier.srcPos.isSynthetic
       && tree.qualifier.tpe.match
@@ -96,7 +93,6 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
         resolveUsage(sym, name, tree.qualifier.tpe)
     else if !ignoreTree(tree) then
       refUsage(sym)
-    refInfos.isAssignment = false
     tree
 
   override def transformLiteral(tree: Literal)(using Context): tree.type =
@@ -132,10 +128,11 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
     tree
 
   override def prepareForAssign(tree: Assign)(using Context): Context =
-    tree.lhs.putAttachment(AssignmentTarget, ()) // don't take LHS reference as a read
+    if tree.lhs.symbol.exists then
+      refInfos.setAssignmentTarget(tree.lhs.symbol)
     ctx
   override def transformAssign(tree: Assign)(using Context): tree.type =
-    tree.lhs.removeAttachment(AssignmentTarget)
+    refInfos.resetAssignmentTarget()
     tree
 
   override def prepareForMatch(tree: Match)(using Context): Context =
@@ -310,13 +307,15 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha
     for annot <- sym.denot.annotations do
       transformAllDeep(annot.tree)
 
-  /** If sym is not an enclosing element with respect to the give context, record the reference
+  /** If sym is not an enclosing element with respect to the given context, record the reference
    *
    *  Also check that every enclosing element is not a synthetic member
    *  of the sym's case class companion module.
+   *
+   *  The LHS of a current Assign is never recorded as a reference (that is, a usage).
    */
   def refUsage(sym: Symbol)(using Context): Unit =
-    if !refInfos.hasRef(sym) then
+    if !refInfos.hasRef(sym) && !refInfos.isAssignmentTarget(sym) then
       val isCase = sym.is(Case) && sym.isClass
       if !ctx.outersIterator.exists: outer =>
         val owner = outer.owner
@@ -505,9 +504,6 @@ object CheckUnused:
   /** Ignore reference. */
   val Ignore = Property.StickyKey[Unit]
 
-  /** Tree is LHS of Assign. */
-  val AssignmentTarget = Property.StickyKey[Unit]
-
   /** Tree is an inlined parameter. */
   val InlinedParameter = Property.StickyKey[Unit]
 
@@ -559,18 +555,18 @@ object CheckUnused:
 
     var inliners = 0 // depth of inline def (not inlined yet)
 
-    // instead of refs.addOne, use refUsage -> addRef to distinguish a read from a write to var
-    var isAssignment = false
+    private var assignmentTarget: Symbol = NoSymbol
+    def isAssignmentTarget(sym: Symbol): Boolean = sym eq assignmentTarget
+    def resetAssignmentTarget(): Unit =
+      assignmentTarget = NoSymbol
+    def setAssignmentTarget(sym: Symbol): Unit =
+      assignmentTarget = sym
+      asss.addOne(sym)
+    // @pre !isAssignmentTarget(sym), see refUsage
     def addRef(sym: Symbol): Unit =
-      if isAssignment then
-        asss.addOne(sym)
-      else
-        refs.addOne(sym)
+      refs.addOne(sym)
     def hasRef(sym: Symbol): Boolean =
-      if isAssignment then
-        asss(sym)
-      else
-        refs(sym)
+      refs(sym)
 
     // currently compiletime.testing is completely erased, so ignore the unit
     var isNullified = false

tests/warn/i24280.scala

@@ -0,0 +1,18 @@
+//> using options -Wunused:all
+
+class Foo {
+  def foo(): Any = {
+    var i = 0 // warn mutated but not read
+    val f = () => i += 1
+    f
+  }
+  def bar(): Any = {
+    var i = 0 // warn
+    val g = () => i = i + 1
+    g
+  }
+  object Select:
+    private var i = 0 // warn
+  class Select:
+    def test = Select.i += 1
+}