From 4219db2e0306b739fc9671bca9c1fac82f4c2013 Mon Sep 17 00:00:00 2001 From: Szymon Rodziewicz Date: Tue, 7 Mar 2023 15:18:52 +0100 Subject: [PATCH 1/8] WUnused: Fix for symbols with synthetic names and unused transparent inlines --- .../tools/dotc/transform/CheckUnused.scala | 27 +++++++++++++++++- .../fatal-warnings/i15503i.scala | 28 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 5e4ed6f6e0df..cc51d9bfbe64 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -10,7 +10,7 @@ import dotty.tools.dotc.core.Decorators.{em, i} import dotty.tools.dotc.core.Flags.* import dotty.tools.dotc.core.Phases.Phase import dotty.tools.dotc.core.StdNames -import dotty.tools.dotc.report +import dotty.tools.dotc.{ast, report} import dotty.tools.dotc.reporting.Message import dotty.tools.dotc.typer.ImportInfo import dotty.tools.dotc.util.{Property, SrcPos} @@ -432,6 +432,20 @@ object CheckUnused: else exists } + + // not report unused transparent inline imports + for { + imp <- imports + sel <- imp.selectors + } { + if unusedImport.contains(sel) then + val tpd.Import(qual, _) = imp + val importedMembers = qual.tpe.member(sel.name).alternatives.map(_.symbol) + val isTransparentAndInline = importedMembers.exists(s => s.is(Transparent) && s.is(Inline)) + if isTransparentAndInline then + unusedImport -= sel + } + // if there's an outer scope if usedInScope.nonEmpty then // we keep the symbols not referencing an import in this scope @@ -450,6 +464,7 @@ object CheckUnused: */ def getUnused(using Context): UnusedResult = popScope() + val sortedImp = if ctx.settings.WunusedHas.imports || ctx.settings.WunusedHas.strictNoImplicitWarn then unusedImport.map(d => d.srcPos -> WarnTypes.Imports).toList @@ -460,6 +475,7 @@ object CheckUnused: localDefInScope .filterNot(d => d.symbol.usedDefContains) .filterNot(d => usedInPosition.exists { case (pos, name) => d.span.contains(pos.span) && name == d.symbol.name}) + .filterNot(d => containsSyntheticSuffix(d.symbol)) .map(d => d.namePos -> WarnTypes.LocalDefs).toList else Nil @@ -467,6 +483,7 @@ object CheckUnused: if ctx.settings.WunusedHas.explicits then explicitParamInScope .filterNot(d => d.symbol.usedDefContains) + .filterNot(d => containsSyntheticSuffix(d.symbol)) .map(d => d.namePos -> WarnTypes.ExplicitParams).toList else Nil @@ -474,6 +491,7 @@ object CheckUnused: if ctx.settings.WunusedHas.implicits then implicitParamInScope .filterNot(d => d.symbol.usedDefContains) + .filterNot(d => containsSyntheticSuffix(d.symbol)) .map(d => d.namePos -> WarnTypes.ImplicitParams).toList else Nil @@ -481,6 +499,7 @@ object CheckUnused: if ctx.settings.WunusedHas.privates then privateDefInScope .filterNot(d => d.symbol.usedDefContains) + .filterNot(d => containsSyntheticSuffix(d.symbol)) .map(d => d.namePos -> WarnTypes.PrivateMembers).toList else Nil @@ -488,6 +507,7 @@ object CheckUnused: if ctx.settings.WunusedHas.patvars then patVarsInScope .filterNot(d => d.symbol.usedDefContains) + .filterNot(d => containsSyntheticSuffix(d.symbol)) .filterNot(d => usedInPosition.exists { case (pos, name) => d.span.contains(pos.span) && name == d.symbol.name}) .map(d => d.namePos -> WarnTypes.PatVars).toList else @@ -500,6 +520,11 @@ object CheckUnused: end getUnused //============================ HELPERS ==================================== + /** + * Heuristic to detect synthetic suffixes in names of symbols + */ + private def containsSyntheticSuffix(symbol: Symbol)(using Context): Boolean = + symbol.name.mangledString.contains("$") /** * Is the the constructor of synthetic package object * Should be ignored as it is always imported/used in package diff --git a/tests/neg-custom-args/fatal-warnings/i15503i.scala b/tests/neg-custom-args/fatal-warnings/i15503i.scala index 3dd4d1fc61e7..9ac2ec5ef622 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503i.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503i.scala @@ -209,6 +209,34 @@ package foo.test.i16925: _ = println(i) // OK } yield () +package foo.test.i16863a: + import scala.quoted.* + def fn(using Quotes) = + val x = Expr(1) + '{ $x + 2 } // OK + +package foo.test.i16863b: + import scala.quoted.* + def fn[A](using Quotes, Type[A]) = // OK + val numeric = Expr.summon[Numeric[A]].getOrElse(???) + '{ $numeric.fromInt(3) } // OK + +package foo.test.i16863c: + import scala.quoted.* + def fn[A](expr: Expr[Any])(using Quotes) = + val imp = expr match + case '{ ${ _ }: a } => Expr.summon[Numeric[a]] // OK + println(imp) + +package foo.test.i16863d: + import scala.quoted.* + import scala.compiletime.asMatchable // OK + def fn[A](using Quotes, Type[A]) = + import quotes.reflect.* + val imp = TypeRepr.of[A].widen.asMatchable match + case Refinement(_,_,_) => () + println(imp) + package foo.test.i16679a: object myPackage: trait CaseClassName[A]: From c36965ce94ff81e18e88294d932a3d9014668f70 Mon Sep 17 00:00:00 2001 From: Szymon Rodziewicz Date: Wed, 8 Mar 2023 13:24:54 +0100 Subject: [PATCH 2/8] Adjust assertions in test --- .../fatal-warnings/i15503-scala2/scala2-t11681.scala | 4 ++-- tests/neg-custom-args/fatal-warnings/i15503b.scala | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/neg-custom-args/fatal-warnings/i15503-scala2/scala2-t11681.scala b/tests/neg-custom-args/fatal-warnings/i15503-scala2/scala2-t11681.scala index f04129a19e48..18aa6879eeba 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503-scala2/scala2-t11681.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503-scala2/scala2-t11681.scala @@ -100,9 +100,9 @@ trait Anonymous { trait Context[A] trait Implicits { def f[A](implicit ctx: Context[A]) = answer // error - def g[A: Context] = answer // error + def g[A: Context] = answer // OK } -class Bound[A: Context] // error +class Bound[A: Context] // OK object Answers { def answer: Int = 42 } diff --git a/tests/neg-custom-args/fatal-warnings/i15503b.scala b/tests/neg-custom-args/fatal-warnings/i15503b.scala index 19bcd01a8dde..8a4a055150f9 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503b.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503b.scala @@ -75,7 +75,7 @@ package foo.scala2.tests: object Types { def l1() = { - object HiObject { def f = this } // error + object HiObject { def f = this } // OK class Hi { // error def f1: Hi = new Hi def f2(x: Hi) = x From 8ff475452f74111be884cea7f18b1b818e6c4c0a Mon Sep 17 00:00:00 2001 From: Szymon Rodziewicz Date: Fri, 10 Mar 2023 17:36:15 +0100 Subject: [PATCH 3/8] Check if import contains transparent inline in registerImport --- .../tools/dotc/transform/CheckUnused.scala | 29 +++++++++---------- .../fatal-warnings/i15503f.scala | 2 +- .../fatal-warnings/i15503g.scala | 4 +-- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index cc51d9bfbe64..9b2fd122f68a 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -10,7 +10,7 @@ import dotty.tools.dotc.core.Decorators.{em, i} import dotty.tools.dotc.core.Flags.* import dotty.tools.dotc.core.Phases.Phase import dotty.tools.dotc.core.StdNames -import dotty.tools.dotc.{ast, report} +import dotty.tools.dotc.report import dotty.tools.dotc.reporting.Message import dotty.tools.dotc.typer.ImportInfo import dotty.tools.dotc.util.{Property, SrcPos} @@ -368,7 +368,7 @@ object CheckUnused: /** Register an import */ def registerImport(imp: tpd.Import)(using Context): Unit = - if !tpd.languageImport(imp.expr).nonEmpty && !imp.isGeneratedByEnum then + if !tpd.languageImport(imp.expr).nonEmpty && !imp.isGeneratedByEnum && !isTransparentAndInline(imp) then impInScope.top += imp unusedImport ++= imp.selectors.filter { s => !shouldSelectorBeReported(imp, s) && !isImportExclusion(s) @@ -433,19 +433,6 @@ object CheckUnused: exists } - // not report unused transparent inline imports - for { - imp <- imports - sel <- imp.selectors - } { - if unusedImport.contains(sel) then - val tpd.Import(qual, _) = imp - val importedMembers = qual.tpe.member(sel.name).alternatives.map(_.symbol) - val isTransparentAndInline = importedMembers.exists(s => s.is(Transparent) && s.is(Inline)) - if isTransparentAndInline then - unusedImport -= sel - } - // if there's an outer scope if usedInScope.nonEmpty then // we keep the symbols not referencing an import in this scope @@ -520,6 +507,18 @@ object CheckUnused: end getUnused //============================ HELPERS ==================================== + + /** + * Checks if import selects a def that is transparent and inline + */ + private def isTransparentAndInline(imp: tpd.Import)(using Context): Boolean = + (for { + sel <- imp.selectors + } yield { + val qual = imp.expr + val importedMembers = qual.tpe.member(sel.name).alternatives.map(_.symbol) + importedMembers.exists(s => s.is(Transparent) && s.is(Inline)) + }).exists(identity) /** * Heuristic to detect synthetic suffixes in names of symbols */ diff --git a/tests/neg-custom-args/fatal-warnings/i15503f.scala b/tests/neg-custom-args/fatal-warnings/i15503f.scala index db695da3490b..d36cd01be74e 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503f.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503f.scala @@ -5,7 +5,7 @@ val default_int = 1 def f1(a: Int) = a // OK def f2(a: Int) = 1 // OK -def f3(a: Int)(using Int) = a // error +def f3(a: Int)(using Int) = a // OK def f4(a: Int)(using Int) = default_int // error def f6(a: Int)(using Int) = summon[Int] // OK def f7(a: Int)(using Int) = summon[Int] + a // OK diff --git a/tests/neg-custom-args/fatal-warnings/i15503g.scala b/tests/neg-custom-args/fatal-warnings/i15503g.scala index d4daea944184..a0822e7e1611 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503g.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503g.scala @@ -5,8 +5,8 @@ val default_int = 1 def f1(a: Int) = a // OK def f2(a: Int) = default_int // error -def f3(a: Int)(using Int) = a // error -def f4(a: Int)(using Int) = default_int // error // error +def f3(a: Int)(using Int) = a // OK +def f4(a: Int)(using Int) = default_int // error def f6(a: Int)(using Int) = summon[Int] // error def f7(a: Int)(using Int) = summon[Int] + a // OK From 56b484dcb62e0d98c6bd0178b4469ed405e212ef Mon Sep 17 00:00:00 2001 From: Szymon Rodziewicz Date: Tue, 28 Mar 2023 17:32:00 +0200 Subject: [PATCH 4/8] Warn for synthetic using/givens with wunused --- compiler/src/dotty/tools/dotc/transform/CheckUnused.scala | 2 +- tests/neg-custom-args/fatal-warnings/i15503f.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 9b2fd122f68a..bf1ec37ebab4 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -478,7 +478,7 @@ object CheckUnused: if ctx.settings.WunusedHas.implicits then implicitParamInScope .filterNot(d => d.symbol.usedDefContains) - .filterNot(d => containsSyntheticSuffix(d.symbol)) + .filterNot(d => containsSyntheticSuffix(d.symbol) && !d.rawMods.is(Given)) .map(d => d.namePos -> WarnTypes.ImplicitParams).toList else Nil diff --git a/tests/neg-custom-args/fatal-warnings/i15503f.scala b/tests/neg-custom-args/fatal-warnings/i15503f.scala index d36cd01be74e..db695da3490b 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503f.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503f.scala @@ -5,7 +5,7 @@ val default_int = 1 def f1(a: Int) = a // OK def f2(a: Int) = 1 // OK -def f3(a: Int)(using Int) = a // OK +def f3(a: Int)(using Int) = a // error def f4(a: Int)(using Int) = default_int // error def f6(a: Int)(using Int) = summon[Int] // OK def f7(a: Int)(using Int) = summon[Int] + a // OK From 00b3d251a97817b253b4bef0c45efee4c03d9867 Mon Sep 17 00:00:00 2001 From: Szymon Rodziewicz Date: Tue, 28 Mar 2023 19:29:09 +0200 Subject: [PATCH 5/8] Wunused: only filter out non-zero span-length givens --- .../src/dotty/tools/dotc/transform/CheckUnused.scala | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index bf1ec37ebab4..665c0b4284ca 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -478,7 +478,7 @@ object CheckUnused: if ctx.settings.WunusedHas.implicits then implicitParamInScope .filterNot(d => d.symbol.usedDefContains) - .filterNot(d => containsSyntheticSuffix(d.symbol) && !d.rawMods.is(Given)) + .filterNot(d => containsSyntheticSuffix(d.symbol) && (!d.rawMods.is(Given) || hasZeroLengthSpan(d.symbol))) .map(d => d.namePos -> WarnTypes.ImplicitParams).toList else Nil @@ -519,11 +519,18 @@ object CheckUnused: val importedMembers = qual.tpe.member(sel.name).alternatives.map(_.symbol) importedMembers.exists(s => s.is(Transparent) && s.is(Inline)) }).exists(identity) + /** * Heuristic to detect synthetic suffixes in names of symbols */ private def containsSyntheticSuffix(symbol: Symbol)(using Context): Boolean = symbol.name.mangledString.contains("$") + + /** + * Heuristic to detect generated symbols by checking if symbol has zero length span in source + */ + private def hasZeroLengthSpan(symbol: Symbol)(using Context): Boolean = + symbol.span.end - symbol.span.start == 0 /** * Is the the constructor of synthetic package object * Should be ignored as it is always imported/used in package From e9077a9ff4385aa79b0d34aab58b87b55749ca96 Mon Sep 17 00:00:00 2001 From: Szymon Rodziewicz Date: Wed, 29 Mar 2023 14:52:46 +0200 Subject: [PATCH 6/8] Skip all symbols with $ in name in Wunused --- compiler/src/dotty/tools/dotc/transform/CheckUnused.scala | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 665c0b4284ca..f960f7b9e60c 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -478,7 +478,7 @@ object CheckUnused: if ctx.settings.WunusedHas.implicits then implicitParamInScope .filterNot(d => d.symbol.usedDefContains) - .filterNot(d => containsSyntheticSuffix(d.symbol) && (!d.rawMods.is(Given) || hasZeroLengthSpan(d.symbol))) + .filterNot(d => containsSyntheticSuffix(d.symbol)) .map(d => d.namePos -> WarnTypes.ImplicitParams).toList else Nil @@ -526,11 +526,6 @@ object CheckUnused: private def containsSyntheticSuffix(symbol: Symbol)(using Context): Boolean = symbol.name.mangledString.contains("$") - /** - * Heuristic to detect generated symbols by checking if symbol has zero length span in source - */ - private def hasZeroLengthSpan(symbol: Symbol)(using Context): Boolean = - symbol.span.end - symbol.span.start == 0 /** * Is the the constructor of synthetic package object * Should be ignored as it is always imported/used in package From a83d0c954a619076aec91e1ed6d141a600960913 Mon Sep 17 00:00:00 2001 From: Szymon Rodziewicz Date: Wed, 29 Mar 2023 15:51:50 +0200 Subject: [PATCH 7/8] Add a failing case with named using to test Wunused:implicits --- tests/neg-custom-args/fatal-warnings/i15503f.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/neg-custom-args/fatal-warnings/i15503f.scala b/tests/neg-custom-args/fatal-warnings/i15503f.scala index db695da3490b..67c595d74f40 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503f.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503f.scala @@ -5,8 +5,9 @@ val default_int = 1 def f1(a: Int) = a // OK def f2(a: Int) = 1 // OK -def f3(a: Int)(using Int) = a // error -def f4(a: Int)(using Int) = default_int // error +def f3(a: Int)(using Int) = a // OK +def f4(a: Int)(using Int) = default_int // OK def f6(a: Int)(using Int) = summon[Int] // OK def f7(a: Int)(using Int) = summon[Int] + a // OK +def f8(a: Int)(using foo: Int) = a // error From 1f9e8693d5b782d11774b1d239fc6c82aa734ebe Mon Sep 17 00:00:00 2001 From: Szymon Rodziewicz Date: Wed, 29 Mar 2023 16:10:21 +0200 Subject: [PATCH 8/8] Replace for with exists in isTransparentInline in WUNused --- compiler/src/dotty/tools/dotc/transform/CheckUnused.scala | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index f960f7b9e60c..d7c88a1fca40 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -512,13 +512,11 @@ object CheckUnused: * Checks if import selects a def that is transparent and inline */ private def isTransparentAndInline(imp: tpd.Import)(using Context): Boolean = - (for { - sel <- imp.selectors - } yield { + imp.selectors.exists { sel => val qual = imp.expr val importedMembers = qual.tpe.member(sel.name).alternatives.map(_.symbol) importedMembers.exists(s => s.is(Transparent) && s.is(Inline)) - }).exists(identity) + } /** * Heuristic to detect synthetic suffixes in names of symbols