Warn if implicit default shadows given

Author: som-snytt

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

@@ -167,6 +167,7 @@ private sealed trait WarningSettings:
   private val WimplausiblePatterns = BooleanSetting(WarningSetting, "Wimplausible-patterns", "Warn if comparison with a pattern value looks like it might always fail.")
   private val WunstableInlineAccessors = BooleanSetting(WarningSetting, "WunstableInlineAccessors", "Warn an inline methods has references to non-stable binary APIs.")
   private val WtoStringInterpolated = BooleanSetting(WarningSetting, "Wtostring-interpolated", "Warn a standard interpolator used toString on a reference type.")
+  private val WrecurseWithDefault = BooleanSetting(WarningSetting, "Wtailrec-default", "Warn when a tail-recursive call uses a default arg.")
   private val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting(
     WarningSetting,
     name = "Wunused",
@@ -309,6 +310,7 @@ private sealed trait WarningSettings:
     def implausiblePatterns(using Context): Boolean = allOr(WimplausiblePatterns)
     def unstableInlineAccessors(using Context): Boolean = allOr(WunstableInlineAccessors)
     def toStringInterpolated(using Context): Boolean = allOr(WtoStringInterpolated)
+    def recurseWithDefault(using Context): Boolean = allOr(WrecurseWithDefault)
     def checkInit(using Context): Boolean = allOr(WcheckInit)
 
 /** -X "Extended" or "Advanced" settings */

compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

@@ -232,6 +232,8 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case UnnecessaryNN // errorNumber: 216
   case ErasedNotPureID // errorNumber: 217
   case IllegalErasedDefID // errorNumber: 218
+  case DefaultShadowsGivenID // errorNumber: 219
+  case TailrecUsedDefaultID // errorNumber: 220
 
   def errorNumber = ordinal - 1
 

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -3652,3 +3652,15 @@ final class IllegalErasedDef(sym: Symbol)(using Context) extends TypeMsg(Illegal
   override protected def explain(using Context): String =
     "Only non-lazy immutable values can be `erased`"
 end IllegalErasedDef
+
+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 it's intended to prefer the given in scope, but you must specify it explicitly."
+
+final class TailrecUsedDefault(using Context) extends TypeMsg(TailrecUsedDefaultID):
+  override protected def msg(using Context): String =
+    i"Recursive call used a default argument."
+  override protected def explain(using Context): String =
+    "Usually it's intended to pass current arguments in a recursion."

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

@@ -10,6 +10,7 @@ import NameKinds.{TailLabelName, TailLocalName, TailTempName}
 import StdNames.nme
 import reporting.*
 import transform.MegaPhase.MiniPhase
+import typer.Applications.UsedDefaults
 import util.LinearSet
 import dotty.tools.uncheckedNN
 
@@ -325,7 +326,10 @@ class TailRec extends MiniPhase {
           method.matches(calledMethod) &&
           enclosingClass.appliedRef.widen <:< prefix.tpe.widenDealias
 
-        if (isRecursiveCall)
+        if isRecursiveCall then
+          if ctx.settings.Whas.recurseWithDefault && tree.args.exists(_.hasAttachment(UsedDefaults)) then
+            report.warning(TailrecUsedDefault(), tree.srcPos)
+
           if (inTailPosition) {
             tailrec.println("Rewriting tail recursive call:  " + tree.span)
             rewrote = true

compiler/src/dotty/tools/dotc/typer/Applications.scala

@@ -5,7 +5,7 @@ package typer
 import core.*
 import ast.{Trees, tpd, untpd, desugar}
 import util.Stats.record
-import util.{SrcPos, NoSourcePosition}
+import util.{Property, SrcPos, NoSourcePosition}
 import Contexts.*
 import Flags.*
 import Symbols.*
@@ -42,6 +42,9 @@ import dotty.tools.dotc.inlines.Inlines
 object Applications {
   import tpd.*
 
+  /** Attachment indicating that an argument in an application was a default arg. */
+  val UsedDefaults = Property.StickyKey[Unit]
+
   def extractorMember(tp: Type, name: Name)(using Context): SingleDenotation =
     tp.member(name).suchThat(sym => sym.info.isParameterless && sym.info.widenExpr.isValueType)
 
@@ -774,6 +777,14 @@ trait Applications extends Compatibility {
               methodType.isImplicitMethod && (applyKind == ApplyKind.Using || failEmptyArgs)
 
             if !defaultArg.isEmpty then
+              if methodType.isImplicitMethod && ctx.mode.is(Mode.ImplicitsEnabled)
+              && inferImplicitArg(formal, appPos.span).tpe.isError == false
+              then
+                report.warning(DefaultShadowsGiven(methodType.paramNames(n)), appPos)
+              else if ctx.settings.Whas.recurseWithDefault && ctx.outersIterator.map(_.owner).toSet(sym)
+              then
+                defaultArg.withAttachment(UsedDefaults, ()) // might warn at tailrec
+
               defaultArg.tpe.widen match
                 case _: MethodOrPoly if testOnly => matchArgs(args1, formals1, n + 1)
                 case _ => matchArgs(args1, addTyped(treeToArg(defaultArg)), n + 1)

tests/neg/19414-desugared.check renamed to 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.

tests/neg/19414-desugared.scala renamed to tests/neg/i19414-desugared.scala

@@ -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.

tests/neg/19414.check renamed to tests/neg/i19414.check

@@ -0,0 +1,30 @@
+//> using options -Wtailrec-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"