From 58930255e8f6d5bbc2950b797c6aa70c5a813c6a Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Tue, 12 Aug 2025 20:13:46 +0200 Subject: [PATCH 1/4] Warn if implicit default shadows given -Wrecurse-with-default warns on self-recursion with default arg. [Cherry-picked 6dac167b2a182194a20b0060918a943ace5c89e6][modified] --- .../tools/dotc/config/ScalaSettings.scala | 2 ++ .../tools/dotc/reporting/ErrorMessageID.scala | 2 ++ .../dotty/tools/dotc/reporting/messages.scala | 12 ++++++++ .../dotty/tools/dotc/transform/TailRec.scala | 10 +++++-- .../dotty/tools/dotc/typer/Applications.scala | 5 ++++ .../tools/scaladoc/tasty/TypesSupport.scala | 6 ++-- ...desugared.check => i19414-desugared.check} | 2 +- ...desugared.scala => i19414-desugared.scala} | 0 tests/neg/{19414.check => i19414.check} | 2 +- tests/neg/{19414.scala => i19414.scala} | 2 +- tests/warn/i23541.scala | 30 +++++++++++++++++++ 11 files changed, 65 insertions(+), 8 deletions(-) rename tests/neg/{19414-desugared.check => i19414-desugared.check} (91%) rename tests/neg/{19414-desugared.scala => i19414-desugared.scala} (100%) rename tests/neg/{19414.check => i19414.check} (91%) rename tests/neg/{19414.scala => i19414.scala} (80%) create mode 100644 tests/warn/i23541.scala diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index ddd184c445f0..c25e5e0db11c 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -175,6 +175,7 @@ private sealed trait WarningSettings: private val WNonUnitStatement = BooleanSetting("-Wnonunit-statement", "Warn when block statements are non-Unit expressions.") private val WenumCommentDiscard = BooleanSetting("-Wenum-comment-discard", "Warn when a comment ambiguously assigned to multiple enum cases is discarded.") private val WtoStringInterpolated = BooleanSetting("-Wtostring-interpolated", "Warn a standard interpolator used toString on a reference type.") + private val WrecurseWithDefault = BooleanSetting("-Wrecurse-with-default", "Warn when a method calls itself with a default argument.") private val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting( name = "-Wunused", helpArg = "warning", @@ -292,6 +293,7 @@ private sealed trait WarningSettings: def nonUnitStatement(using Context): Boolean = allOr(WNonUnitStatement) def enumCommentDiscard(using Context): Boolean = allOr(WenumCommentDiscard) def toStringInterpolated(using Context): Boolean = allOr(WtoStringInterpolated) + def recurseWithDefault(using Context): Boolean = allOr(WrecurseWithDefault) def checkInit(using Context): Boolean = allOr(YcheckInit) /** -X "Extended" or "Advanced" settings */ diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala index f8befc5a19df..7b4dca438bd8 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala @@ -233,6 +233,8 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe case ErasedNotPureID // errorNumber: 217 case IllegalErasedDefID // errorNumber: 218 case CannotInstantiateQuotedTypeVarID // errorNumber: 219 + case DefaultShadowsGivenID // errorNumber: 220 + case RecurseWithDefaultID // errorNumber: 221 def errorNumber = ordinal - 1 diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 83ccb45780d4..0e2d64266dbe 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -3307,3 +3307,15 @@ final class NamedPatternNotApplicable(selectorType: Type)(using Context) extends i"Named patterns cannot be used with $selectorType, because it is not a named tuple or case class" override protected def explain(using Context): String = "" + +final class DefaultShadowsGiven(name: Name)(using Context) extends TypeMsg(DefaultShadowsGivenID): + override protected def msg(using Context): String = + i"Argument for implicit parameter $name was supplied using a default argument." + override protected def explain(using Context): String = + "Usually the given in scope is intended, but you must specify it after explicit `using`." + +final class RecurseWithDefault(name: Name)(using Context) extends TypeMsg(RecurseWithDefaultID): + override protected def msg(using Context): String = + i"Recursive call used a default argument for parameter $name." + override protected def explain(using Context): String = + "It's more explicit to pass current or modified arguments in a recursion." \ No newline at end of file diff --git a/compiler/src/dotty/tools/dotc/transform/TailRec.scala b/compiler/src/dotty/tools/dotc/transform/TailRec.scala index a9cfd40967e2..8eb6c069d2e4 100644 --- a/compiler/src/dotty/tools/dotc/transform/TailRec.scala +++ b/compiler/src/dotty/tools/dotc/transform/TailRec.scala @@ -4,9 +4,9 @@ package transform import ast.{TreeTypeMap, tpd} import config.Printers.tailrec import core.* -import Contexts.*, Flags.*, Symbols.*, Decorators.em +import Contexts.*, Flags.*, Symbols.*, Decorators.* import Constants.Constant -import NameKinds.{TailLabelName, TailLocalName, TailTempName} +import NameKinds.{DefaultGetterName, TailLabelName, TailLocalName, TailTempName} import StdNames.nme import reporting.* import transform.MegaPhase.MiniPhase @@ -323,7 +323,11 @@ class TailRec extends MiniPhase { method.matches(calledMethod) && enclosingClass.appliedRef.widen <:< prefix.tpe.widenDealias - if (isRecursiveCall) + if isRecursiveCall then + if ctx.settings.Whas.recurseWithDefault then + if tree.args.exists(_.symbol.name.is(DefaultGetterName)) then + report.warning(RecurseWithDefault(), tree.srcPos) + if (inTailPosition) { tailrec.println("Rewriting tail recursive call: " + tree.span) rewrote = true diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index d2fd7a928eb0..38d5bcf3fe9b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -674,6 +674,11 @@ trait Applications extends Compatibility { def implicitArg = implicitArgTree(formal, appPos.span) if !defaultArg.isEmpty then + if methodType.isImplicitMethod && ctx.mode.is(Mode.ImplicitsEnabled) + && !inferImplicitArg(formal, appPos.span).tpe.isError + then + report.warning(DefaultShadowsGiven(methodType.paramNames(n)), appPos) + defaultArg.tpe.widen match case _: MethodOrPoly if testOnly => matchArgs(args1, formals1, n + 1) case _ => matchArgs(args1, addTyped(treeToArg(defaultArg)), n + 1) diff --git a/scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala b/scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala index 774e775f4e53..ffd161738421 100644 --- a/scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala +++ b/scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala @@ -4,9 +4,10 @@ package tasty import scala.jdk.CollectionConverters._ import scala.quoted._ +import scala.annotation.* -import NameNormalizer._ -import SyntheticsSupport._ +import NameNormalizer.* +import SyntheticsSupport.* trait TypesSupport: self: TastyParser => @@ -76,6 +77,7 @@ trait TypesSupport: case tpe => inner(tpe) // TODO #23 add support for all types signatures that make sense + @nowarn("id=E219") private def inner( using Quotes, )( diff --git a/tests/neg/19414-desugared.check b/tests/neg/i19414-desugared.check similarity index 91% rename from tests/neg/19414-desugared.check rename to tests/neg/i19414-desugared.check index cc51ee471553..72a3a5eabd37 100644 --- a/tests/neg/19414-desugared.check +++ b/tests/neg/i19414-desugared.check @@ -1,4 +1,4 @@ --- [E172] Type Error: tests/neg/19414-desugared.scala:22:34 ------------------------------------------------------------ +-- [E172] Type Error: tests/neg/i19414-desugared.scala:22:34 ----------------------------------------------------------- 22 | summon[BodySerializer[JsObject]] // error: Ambiguous given instances | ^ |No best given instance of type BodySerializer[JsObject] was found for parameter x of method summon in object Predef. diff --git a/tests/neg/19414-desugared.scala b/tests/neg/i19414-desugared.scala similarity index 100% rename from tests/neg/19414-desugared.scala rename to tests/neg/i19414-desugared.scala diff --git a/tests/neg/19414.check b/tests/neg/i19414.check similarity index 91% rename from tests/neg/19414.check rename to tests/neg/i19414.check index 016e3942c825..10bc939494c6 100644 --- a/tests/neg/19414.check +++ b/tests/neg/i19414.check @@ -1,4 +1,4 @@ --- [E172] Type Error: tests/neg/19414.scala:15:34 ---------------------------------------------------------------------- +-- [E172] Type Error: tests/neg/i19414.scala:15:34 --------------------------------------------------------------------- 15 | summon[BodySerializer[JsObject]] // error: Ambiguous given instances | ^ |No best given instance of type BodySerializer[JsObject] was found for parameter x of method summon in object Predef. diff --git a/tests/neg/19414.scala b/tests/neg/i19414.scala similarity index 80% rename from tests/neg/19414.scala rename to tests/neg/i19414.scala index bb275ad943b7..8843441e81f2 100644 --- a/tests/neg/19414.scala +++ b/tests/neg/i19414.scala @@ -9,7 +9,7 @@ class Printer given Writer[JsValue] = ??? given Writer[JsObject] = ??? -given [B: Writer](using printer: Printer = new Printer): BodySerializer[B] = ??? +given [B: Writer] => (printer: Printer = new Printer) => BodySerializer[B] = ??? def f: Unit = summon[BodySerializer[JsObject]] // error: Ambiguous given instances diff --git a/tests/warn/i23541.scala b/tests/warn/i23541.scala new file mode 100644 index 000000000000..7f374b421ba3 --- /dev/null +++ b/tests/warn/i23541.scala @@ -0,0 +1,30 @@ +//> using options -Wrecurse-with-default + +def fun(x: Int)(using p: Int, q: Int = 0): Int = + if x <= 0 then p * q + else fun(x - 1)(using p = p + x) // warn recurse uses default (instead of given passed down the stack) + +def gun(x: Int)(p: Int, q: Int = 0): Int = + if x <= 0 then p * q + else gun(x - 1)(p = p + x) // warn recurse uses default (value not passed down the stack) + +def nested(using x: Int, y: Int = 42): Int = + def f: Int = nested(using x) // nowarn only self-recursive tailrec is eligible for warning + f + +def f(using s: String, i: Int = 1): String = s * i +def g(using s: String)(using i: Int = 1): String = s * i + +@main def Test = + println(fun(3)(using p = 0, q = 1)) + locally: + given String = "ab" + println(f) // prints "ab" + println(g) // prints "ab" + locally: + println(f(using s = "ab")) // prints "ab" + println(g(using s = "ab")) // prints "ab" + locally: + given Int = 2 + println(f(using s = "ab")) // warn uses default instead of given // prints "ab" + println(g(using s = "ab")) // prints "abab" From 7331d29533b10fa6950cb42a769f82dcd54bb656 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 23 Jul 2025 14:10:51 -0700 Subject: [PATCH 2/4] Use explicit args in recursion --- .../tools/scaladoc/tasty/TypesSupport.scala | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala b/scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala index ffd161738421..8cef8af12604 100644 --- a/scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala +++ b/scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala @@ -77,7 +77,6 @@ trait TypesSupport: case tpe => inner(tpe) // TODO #23 add support for all types signatures that make sense - @nowarn("id=E219") private def inner( using Quotes, )( @@ -143,24 +142,24 @@ trait TypesSupport: .reduceLeftOption((acc: SSignature, elem: SSignature) => acc ++ plain(", ").l ++ elem).getOrElse(List()) ++ plain(")").l - def parseRefinedElem(name: String, info: TypeRepr, polyTyped: SSignature = Nil): SSignature = ( info match { + def parseRefinedElem(name: String, info: TypeRepr, polyTyped: SSignature = Nil): SSignature = + val ssig = info match case m: MethodType => { val paramList = getParamList(m) keyword("def ").l ++ plain(name).l ++ polyTyped ++ paramList ++ plain(": ").l ++ inner(m.resType) } - case t: PolyType => { + case t: PolyType => val paramBounds = getParamBounds(t) - val parsedMethod = parseRefinedElem(name, t.resType) - if (!paramBounds.isEmpty){ + if !paramBounds.isEmpty then parseRefinedElem(name, t.resType, plain("[").l ++ paramBounds ++ plain("]").l) - } else parseRefinedElem(name, t.resType) - } + else parseRefinedElem(name, t.resType, polyTyped = Nil) case ByNameType(tp) => keyword("def ").l ++ plain(s"$name: ").l ++ inner(tp) case t: TypeBounds => keyword("type ").l ++ plain(name).l ++ inner(t) case t: TypeRef => keyword("val ").l ++ plain(s"$name: ").l ++ inner(t) case t: TermRef => keyword("val ").l ++ plain(s"$name: ").l ++ inner(t) case other => noSupported(s"Not supported type in refinement $info") - } ) ++ plain("; ").l + + ssig ++ plain("; ").l def parsePolyFunction(info: TypeRepr): SSignature = info match { case t: PolyType => @@ -227,6 +226,7 @@ trait TypesSupport: }) ++ plain("]").l case tp @ TypeRef(qual, typeName) => + inline def wrapping = shouldWrapInParens(inner = qual, outer = tp, isLeft = true) qual match { case r: RecursiveThis => tpe(s"this.$typeName").l case t if skipPrefix(t, elideThis) => @@ -247,17 +247,17 @@ trait TypesSupport: case _ => tpe(tp.typeSymbol) case Some(_) => tpe(tp.typeSymbol) case None => - val sig = inParens(inner(qual)(using skipTypeSuffix = true), shouldWrapInParens(qual, tp, true)) + val sig = inParens(inner(qual)(using indent = indent, skipTypeSuffix = true), wrapping) sig ++ plain(".").l ++ tpe(tp.typeSymbol) case _ => - val sig = inParens(inner(qual), shouldWrapInParens(qual, tp, true)) + val sig = inParens(inner(qual, skipThisTypePrefix), wrapping) sig ++ keyword("#").l ++ tpe(tp.typeSymbol) } case tr @ TermRef(qual, typeName) => val prefix = qual match case t if skipPrefix(t, elideThis) => Nil - case tp => inner(tp)(using skipTypeSuffix = true) ++ plain(".").l + case tp => inner(tp)(using indent = indent, skipTypeSuffix = true) ++ plain(".").l val suffix = if skipTypeSuffix then Nil else List(plain("."), keyword("type")) val typeSig = tr.termSymbol.tree match case vd: ValDef if tr.termSymbol.flags.is(Flags.Module) => @@ -276,9 +276,9 @@ trait TypesSupport: val spaces = " " * (indent) val casesTexts = cases.flatMap { case MatchCase(from, to) => - keyword(caseSpaces + "case ").l ++ inner(from) ++ keyword(" => ").l ++ inner(to)(using indent = indent + 2) ++ plain("\n").l + keyword(caseSpaces + "case ").l ++ inner(from) ++ keyword(" => ").l ++ inner(to)(using indent = indent + 2, skipTypeSuffix = skipTypeSuffix) ++ plain("\n").l case TypeLambda(_, _, MatchCase(from, to)) => - keyword(caseSpaces + "case ").l ++ inner(from) ++ keyword(" => ").l ++ inner(to)(using indent = indent + 2) ++ plain("\n").l + keyword(caseSpaces + "case ").l ++ inner(from) ++ keyword(" => ").l ++ inner(to)(using indent = indent + 2, skipTypeSuffix = skipTypeSuffix) ++ plain("\n").l } inner(sc) ++ keyword(" match ").l ++ plain("{\n").l ++ casesTexts ++ plain(spaces + "}").l From b6acf376bc65ba794719a8255f9f48182251aa0b Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Tue, 12 Aug 2025 20:20:29 +0200 Subject: [PATCH 3/4] Use explicit args in recursion [Cherry-picked 4cf87b07b8a11fdca834b2439e65d96f775666b1][modified] From 2eb4a9f5d9e814959c2437dcfd162d9656c25300 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 23 Jul 2025 15:05:13 -0700 Subject: [PATCH 4/4] Name the parameter using a default [Cherry-picked 8ba7cd38423b645ed6ef333f62371a34b9db8fde] --- .../dotty/tools/dotc/transform/TailRec.scala | 7 +++++-- tests/warn/i23541.check | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 tests/warn/i23541.check diff --git a/compiler/src/dotty/tools/dotc/transform/TailRec.scala b/compiler/src/dotty/tools/dotc/transform/TailRec.scala index 8eb6c069d2e4..b2be8d4651f0 100644 --- a/compiler/src/dotty/tools/dotc/transform/TailRec.scala +++ b/compiler/src/dotty/tools/dotc/transform/TailRec.scala @@ -325,8 +325,11 @@ class TailRec extends MiniPhase { if isRecursiveCall then if ctx.settings.Whas.recurseWithDefault then - if tree.args.exists(_.symbol.name.is(DefaultGetterName)) then - report.warning(RecurseWithDefault(), tree.srcPos) + tree.args.find(_.symbol.name.is(DefaultGetterName)) match + case Some(arg) => + val DefaultGetterName(_, index) = arg.symbol.name: @unchecked + report.warning(RecurseWithDefault(calledMethod.info.firstParamNames(index)), tree.srcPos) + case _ => if (inTailPosition) { tailrec.println("Rewriting tail recursive call: " + tree.span) diff --git a/tests/warn/i23541.check b/tests/warn/i23541.check new file mode 100644 index 000000000000..64ffbd9808d2 --- /dev/null +++ b/tests/warn/i23541.check @@ -0,0 +1,18 @@ +-- [E220] Type Warning: tests/warn/i23541.scala:29:13 ------------------------------------------------------------------ +29 | println(f(using s = "ab")) // warn uses default instead of given // prints "ab" + | ^^^^^^^^^^^^^^^^^ + | Argument for implicit parameter i was supplied using a default argument. + | + | longer explanation available when compiling with `-explain` +-- [E221] Type Warning: tests/warn/i23541.scala:5:17 ------------------------------------------------------------------- +5 | else fun(x - 1)(using p = p + x) // warn recurse uses default (instead of given passed down the stack) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | Recursive call used a default argument for parameter q. + | + | longer explanation available when compiling with `-explain` +-- [E221] Type Warning: tests/warn/i23541.scala:9:17 ------------------------------------------------------------------- +9 | else gun(x - 1)(p = p + x) // warn recurse uses default (value not passed down the stack) + | ^^^^^^^^^^^^^^^^^^^^^ + | Recursive call used a default argument for parameter q. + | + | longer explanation available when compiling with `-explain`