From 0ebb4db4d769688cb70b7f6b5c4166fceca93190 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 1 Apr 2025 08:32:55 -0700 Subject: [PATCH 1/6] No warn implicit param of overriding method --- .../tools/dotc/transform/CheckUnused.scala | 1 + tests/warn/i15503f.scala | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index ecd59a40619e..587a62591dfb 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -614,6 +614,7 @@ object CheckUnused: || sym.info.isInstanceOf[RefinedType] // can't be expressed as a context bound if ctx.settings.WunusedHas.implicits && !infos.skip(m) + && !m.nextOverriddenSymbol.exists && !allowed then if m.isPrimaryConstructor then diff --git a/tests/warn/i15503f.scala b/tests/warn/i15503f.scala index e134c2ded422..94d264985c26 100644 --- a/tests/warn/i15503f.scala +++ b/tests/warn/i15503f.scala @@ -42,7 +42,7 @@ object ExampleWithoutWith: case '{ ${Expr(opt)} : Some[T] } => Some(opt) case _ => None -//absolving names on matches of quote trees requires consulting non-abstract types in QuotesImpl +//nowarning names on matches of quote trees requires consulting non-abstract types in QuotesImpl object Unmatched: import scala.quoted.* def transform[T](e: Expr[T])(using Quotes): Expr[T] = @@ -84,3 +84,18 @@ package givens: given namely: (x: X) => Y: // warn protected param to given class def doY = "8" end givens + +object i22895: + trait Test[F[_], Ev] { + def apply[A, B](fa: F[A])(f: A => B)(using ev: Ev): F[B] + } + given testId: Test[[a] =>> a, Unit] = + new Test[[a] =>> a, Unit] { + def apply[A, B](fa: A)(f: A => B)(using ev: Unit): B = f(fa) // nowarn override + } + class C: + def f(using s: String) = s.toInt + class D(i: Int) extends C: + override def f(using String) = compute(i) // nowarn override + def g(using sss: String) = compute(i) // warn + def compute(i: Int) = i * 42 // returning a class param is deemed trivial, make it non-trivial From 8253ee23f00e81d53794a3633251637a47161d12 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 1 Apr 2025 10:48:29 -0700 Subject: [PATCH 2/6] Check Override before looking for overridden --- .../dotty/tools/dotc/transform/CheckUnused.scala | 2 +- tests/warn/i22896/J.java | 6 ++++++ tests/warn/i22896/s.scala | 13 +++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 tests/warn/i22896/J.java create mode 100644 tests/warn/i22896/s.scala diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 587a62591dfb..303bb0a42e86 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -924,7 +924,7 @@ object CheckUnused: !m.isTerm || m.isSelfSym || m.is(Method) && (m.owner == defn.AnyClass || m.owner == defn.ObjectClass) def isEffectivelyPrivate(using Context): Boolean = sym.is(Private, butNot = ParamAccessor) - || sym.owner.isAnonymousClass && !sym.nextOverriddenSymbol.exists + || sym.owner.isAnonymousClass && !sym.is(Override) && !sym.nextOverriddenSymbol.exists // pick the symbol the user wrote for purposes of tracking inline def userSymbol(using Context): Symbol= if sym.denot.is(ModuleClass) then sym.denot.companionModule else sym diff --git a/tests/warn/i22896/J.java b/tests/warn/i22896/J.java new file mode 100644 index 000000000000..d110dd16c8fd --- /dev/null +++ b/tests/warn/i22896/J.java @@ -0,0 +1,6 @@ + +public class J { + private int i = 42; + public int i() { return 27; } + public String i(int j) { return "hello, world"; } +} diff --git a/tests/warn/i22896/s.scala b/tests/warn/i22896/s.scala new file mode 100644 index 000000000000..678c77a9bb3e --- /dev/null +++ b/tests/warn/i22896/s.scala @@ -0,0 +1,13 @@ +//> using options -Werror -Wunused:privates + +def f = + new J: + override val i = -1 // nowarn, trust override + +def g = + new J: + override def i = -1 // nowarn, trust override + +def h = + new J: + override def i() = -1 // nowarn correctly matches signature From 1da65992f886c358f26e53d7b1e91bdc1b175fd0 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 2 Apr 2025 08:45:22 -0700 Subject: [PATCH 3/6] Always trust override modifier --- compiler/src/dotty/tools/dotc/transform/CheckUnused.scala | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 303bb0a42e86..119eeff599d0 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -590,7 +590,7 @@ object CheckUnused: end checkExplicit // begin if !infos.skip(m) - && !m.nextOverriddenSymbol.exists + && !m.isEffectivelyOverride && !allowed then checkExplicit() @@ -614,7 +614,7 @@ object CheckUnused: || sym.info.isInstanceOf[RefinedType] // can't be expressed as a context bound if ctx.settings.WunusedHas.implicits && !infos.skip(m) - && !m.nextOverriddenSymbol.exists + && !m.isEffectivelyOverride && !allowed then if m.isPrimaryConstructor then @@ -924,7 +924,9 @@ object CheckUnused: !m.isTerm || m.isSelfSym || m.is(Method) && (m.owner == defn.AnyClass || m.owner == defn.ObjectClass) def isEffectivelyPrivate(using Context): Boolean = sym.is(Private, butNot = ParamAccessor) - || sym.owner.isAnonymousClass && !sym.is(Override) && !sym.nextOverriddenSymbol.exists + || sym.owner.isAnonymousClass && !sym.isEffectivelyOverride + def isEffectivelyOverride(using Context): Boolean = + sym.is(Override) || sym.nextOverriddenSymbol.exists // pick the symbol the user wrote for purposes of tracking inline def userSymbol(using Context): Symbol= if sym.denot.is(ModuleClass) then sym.denot.companionModule else sym From 819586f10409b6f5357d2f15ac014e1f09259e69 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 2 Apr 2025 08:53:24 -0700 Subject: [PATCH 4/6] Collectivize contextual param in extensions --- .../tools/dotc/transform/CheckUnused.scala | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 119eeff599d0..14cbb2f9569b 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -892,43 +892,43 @@ object CheckUnused: inline def exists(p: Name => Boolean): Boolean = nm.ne(nme.NO_NAME) && p(nm) inline def isWildcard: Boolean = nm == nme.WILDCARD || nm.is(WildcardParamName) - extension (tp: Type) - def importPrefix(using Context): Type = tp match + extension (tp: Type)(using Context) + def importPrefix: Type = tp match case tp: NamedType => tp.prefix case tp: ClassInfo => tp.prefix case tp: TypeProxy => tp.superType.normalizedPrefix case _ => NoType - def underlyingPrefix(using Context): Type = tp match + def underlyingPrefix: Type = tp match case tp: NamedType => tp.prefix case tp: ClassInfo => tp.prefix case tp: TypeProxy => tp.underlying.underlyingPrefix case _ => NoType - def skipPackageObject(using Context): Type = + def skipPackageObject: Type = if tp.typeSymbol.isPackageObject then tp.underlyingPrefix else tp - def underlying(using Context): Type = tp match + def underlying: Type = tp match case tp: TypeProxy => tp.underlying case _ => tp private val serializationNames: Set[TermName] = Set("readResolve", "readObject", "readObjectNoData", "writeObject", "writeReplace").map(termName(_)) - extension (sym: Symbol) - def isSerializationSupport(using Context): Boolean = + extension (sym: Symbol)(using Context) + def isSerializationSupport: Boolean = sym.is(Method) && serializationNames(sym.name.toTermName) && sym.owner.isClass && sym.owner.derivesFrom(defn.JavaSerializableClass) - def isCanEqual(using Context): Boolean = + def isCanEqual: Boolean = sym.isOneOf(GivenOrImplicit) && sym.info.finalResultType.baseClasses.exists(_.derivesFrom(defn.CanEqualClass)) - def isMarkerTrait(using Context): Boolean = + def isMarkerTrait: Boolean = sym.isClass && sym.info.allMembers.forall: d => val m = d.symbol !m.isTerm || m.isSelfSym || m.is(Method) && (m.owner == defn.AnyClass || m.owner == defn.ObjectClass) - def isEffectivelyPrivate(using Context): Boolean = + def isEffectivelyPrivate: Boolean = sym.is(Private, butNot = ParamAccessor) || sym.owner.isAnonymousClass && !sym.isEffectivelyOverride - def isEffectivelyOverride(using Context): Boolean = + def isEffectivelyOverride: Boolean = sym.is(Override) || sym.nextOverriddenSymbol.exists // pick the symbol the user wrote for purposes of tracking - inline def userSymbol(using Context): Symbol= + inline def userSymbol: Symbol= if sym.denot.is(ModuleClass) then sym.denot.companionModule else sym extension (sel: ImportSelector) @@ -942,13 +942,13 @@ object CheckUnused: case untpd.Ident(nme.WILDCARD) => true case _ => false - extension (imp: Import) + extension (imp: Import)(using Context) /** Is it the first import clause in a statement? `a.x` in `import a.x, b.{y, z}` */ - def isPrimaryClause(using Context): Boolean = + def isPrimaryClause: Boolean = imp.srcPos.span.pointDelta > 0 // primary clause starts at `import` keyword with point at clause proper /** Generated import of cases from enum companion. */ - def isGeneratedByEnum(using Context): Boolean = + def isGeneratedByEnum: Boolean = imp.symbol.exists && imp.symbol.owner.is(Enum, butNot = Case) /** Under -Wunused:strict-no-implicit-warn, avoid false positives @@ -956,7 +956,7 @@ 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 = + def isLoose(sel: ImportSelector): Boolean = if ctx.settings.WunusedHas.strictNoImplicitWarn then if sel.isWildcard || imp.expr.tpe.member(sel.name.toTermName).hasAltWith(_.symbol.isOneOf(GivenOrImplicit)) From 94defc2bb253663c839ef574af895a2647f5e796 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 2 Apr 2025 11:03:55 -0700 Subject: [PATCH 5/6] Prefer allOverriddenSymbols.hasNext --- compiler/src/dotty/tools/dotc/transform/CheckUnused.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 14cbb2f9569b..70596a5e44a5 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -926,7 +926,7 @@ object CheckUnused: sym.is(Private, butNot = ParamAccessor) || sym.owner.isAnonymousClass && !sym.isEffectivelyOverride def isEffectivelyOverride: Boolean = - sym.is(Override) || sym.nextOverriddenSymbol.exists + sym.is(Override) || sym.allOverriddenSymbols.hasNext // 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 From f8d681fb9cc8be0210662566d16c6d299bce7b0e Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 2 Apr 2025 12:06:58 -0700 Subject: [PATCH 6/6] Consult self type for overrides --- .../src/dotty/tools/dotc/core/SymDenotations.scala | 2 +- .../src/dotty/tools/dotc/transform/CheckUnused.scala | 8 +++++++- tests/warn/unused-params.scala | 10 +++++++++- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index e7eda037117c..6c7b8dcef94a 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1953,7 +1953,7 @@ object SymDenotations { case _ => NoSymbol /** The explicitly given self type (self types of modules are assumed to be - * explcitly given here). + * explicitly given here). */ def givenSelfType(using Context): Type = classInfo.selfInfo match { case tp: Type => tp diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 70596a5e44a5..7663467a3997 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -926,7 +926,13 @@ object CheckUnused: sym.is(Private, butNot = ParamAccessor) || sym.owner.isAnonymousClass && !sym.isEffectivelyOverride def isEffectivelyOverride: Boolean = - sym.is(Override) || sym.allOverriddenSymbols.hasNext + sym.is(Override) + || + sym.canMatchInheritedSymbols && { // inline allOverriddenSymbols using owner.info or thisType + val owner = sym.owner.asClass + val base = if owner.classInfo.selfInfo != NoType then owner.thisType else owner.info + base.baseClasses.drop(1).iterator.exists(sym.overriddenSymbol(_).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 diff --git a/tests/warn/unused-params.scala b/tests/warn/unused-params.scala index 3266f3957247..9136da6425a5 100644 --- a/tests/warn/unused-params.scala +++ b/tests/warn/unused-params.scala @@ -122,7 +122,7 @@ trait BadMix { self: InterFace => a } override def call(a: Int, - XXXX: String, // warn no longer excused because required by superclass + XXXX: String, // no warn still excused because override (required by self type) c: Double): Int = { println(c) a @@ -135,6 +135,14 @@ trait BadMix { self: InterFace => def i(implicit s: String) = answer // warn } +trait ImplFace: + self: InterFace => + def call(a: Int, + b: String, // no warn required by self type + c: Double): Int = + println(c) + a + class Unequal { override def equals(other: Any) = toString.nonEmpty // no warn (override) }