Skip to content

Warn if implicit default shadows given #23559

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 */
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 12 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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."
6 changes: 5 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/TailRec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
13 changes: 12 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.*
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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.exists(_.owner == 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)
Expand Down
11 changes: 6 additions & 5 deletions scaladoc/src/dotty/tools/scaladoc/tasty/TypesSupport.scala
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package dotty.tools.scaladoc
package tasty

import scala.jdk.CollectionConverters._

import scala.quoted._
import scala.annotation.*
import scala.jdk.CollectionConverters.*
import scala.quoted.*
import scala.util.control.NonFatal

import NameNormalizer._
import SyntheticsSupport._
import NameNormalizer.*
import SyntheticsSupport.*

trait TypesSupport:
self: TastyParser =>
Expand Down Expand Up @@ -81,6 +81,7 @@ trait TypesSupport:
case tpe => inner(tpe, skipThisTypePrefix)

// TODO #23 add support for all types signatures that make sense
@nowarn("id=E219")
private def inner(
using Quotes,
)(
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion tests/neg/19414.check → tests/neg/i19414.check
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
File renamed without changes.
30 changes: 30 additions & 0 deletions tests/warn/i23541.scala
Original file line number Diff line number Diff line change
@@ -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"
Loading