From 48e34933cc6fef265a70e0577cf98c5912ab1542 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 11 Apr 2025 05:15:45 -0700 Subject: [PATCH] Improve checking LHS of Assign --- .../tools/dotc/transform/CheckUnused.scala | 28 +++++++++++++------ tests/warn/i15503a.scala | 8 ++++++ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 7663467a3997..0f9583558669 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -52,6 +52,7 @@ 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 = @@ -68,10 +69,12 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha resolveUsage(tree.symbol, tree.name, tree.typeOpt.importPrefix.skipPackageObject) 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 @@ -92,6 +95,7 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha resolveUsage(tree.symbol, name, tree.qualifier.tpe) else if !ignoreTree(tree) then refUsage(tree.symbol) + refInfos.isAssignment = false tree override def transformLiteral(tree: Literal)(using Context): tree.type = @@ -113,13 +117,10 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha ctx override def prepareForAssign(tree: Assign)(using Context): Context = - tree.lhs.putAttachment(Ignore, ()) // don't take LHS reference as a read + tree.lhs.putAttachment(AssignmentTarget, ()) // don't take LHS reference as a read ctx override def transformAssign(tree: Assign)(using Context): tree.type = - tree.lhs.removeAttachment(Ignore) - val sym = tree.lhs.symbol - if sym.exists then - refInfos.asss.addOne(sym) + tree.lhs.removeAttachment(AssignmentTarget) tree override def prepareForMatch(tree: Match)(using Context): Context = @@ -276,7 +277,7 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha // 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) + refInfos.addRef(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 then be correct. @@ -335,7 +336,7 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha 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. + // Selections C.this.toString should be already excluded, but backstopped here for eq, etc. 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. @@ -404,7 +405,7 @@ class CheckUnused private (phaseMode: PhaseMode, suffix: String) extends MiniPha candidate = cur end while // record usage and possibly an import - refInfos.refs.addOne(sym) + refInfos.addRef(sym) if candidate != NoContext && candidate.isImportContext && importer != null then refInfos.sels.put(importer, ()) // possibly record that we have performed this look-up @@ -443,6 +444,9 @@ object CheckUnused: /** Ignore reference. */ val Ignore = Property.StickyKey[Unit] + /** Tree is LHS of Assign. */ + val AssignmentTarget = Property.StickyKey[Unit] + class PostTyper extends CheckUnused(PhaseMode.Aggregate, "PostTyper") class PostInlining extends CheckUnused(PhaseMode.Report, "PostInlining") @@ -487,6 +491,14 @@ object CheckUnused: val inlined = Stack.empty[SrcPos] // enclosing call.srcPos of inlined code (expansions) var inliners = 0 // depth of inline def (not inlined yet) + + // instead of refs.addOne, use addRef to distinguish a read from a write to var + var isAssignment = false + def addRef(sym: Symbol): Unit = + if isAssignment then + asss.addOne(sym) + else + refs.addOne(sym) end RefInfos // Symbols already resolved in the given Context (with name and prefix of lookup). diff --git a/tests/warn/i15503a.scala b/tests/warn/i15503a.scala index 884261e140a7..8fc97888b584 100644 --- a/tests/warn/i15503a.scala +++ b/tests/warn/i15503a.scala @@ -460,3 +460,11 @@ package ancient: } } end ancient + +object `i22970 assign lhs was ignored`: + object X: + var global = 0 + object Main: + import X.global // no warn + def main(args: Array[String]): Unit = + global = 1