Skip to content

Fix numeric implicit args #902

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

Merged
merged 3 commits into from
Nov 9, 2015
Merged
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
5 changes: 4 additions & 1 deletion src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ object Types {
}

/** Is this type a primitive value type which can be widened to the primitive value type `that`? */
def isValueSubType(that: Type)(implicit ctx: Context) = widenExpr match {
def isValueSubType(that: Type)(implicit ctx: Context) = widen match {
case self: TypeRef if defn.ScalaValueClasses contains self.symbol =>
that.widenExpr match {
case that: TypeRef if defn.ScalaValueClasses contains that.symbol =>
Expand All @@ -618,6 +618,9 @@ object Types {
false
}

def relaxed_<:<(that: Type)(implicit ctx: Context) =
(this <:< that) || (this isValueSubType that)

/** Is this type a legal type for a member that overrides another
* member of type `that`? This is the same as `<:<`, except that
* the types ()T and => T are identified, and T is seen as overriding
Expand Down
30 changes: 26 additions & 4 deletions src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,19 @@ object Implicits {
/** Return those references in `refs` that are compatible with type `pt`. */
protected def filterMatching(pt: Type)(implicit ctx: Context): List[TermRef] = track("filterMatching") {

def refMatches(ref: TermRef)(implicit ctx: Context) = {
def refMatches(ref: TermRef)(implicit ctx: Context) = /*ctx.traceIndented(i"refMatches $ref $pt")*/ {

def discardForView(tpw: Type, argType: Type): Boolean = tpw match {
case mt: MethodType =>
mt.isImplicit ||
mt.paramTypes.length != 1 ||
!(argType <:< mt.paramTypes.head)(ctx.fresh.setExploreTyperState)
!(argType relaxed_<:< mt.paramTypes.head)(ctx.fresh.setExploreTyperState)
case poly: PolyType =>
poly.resultType match {
case mt: MethodType =>
mt.isImplicit ||
mt.paramTypes.length != 1 ||
!(argType <:< wildApprox(mt.paramTypes.head)(ctx.fresh.setExploreTyperState))
!(argType relaxed_<:< wildApprox(mt.paramTypes.head)(ctx.fresh.setExploreTyperState))
case rtp =>
discardForView(wildApprox(rtp), argType)
}
Expand Down Expand Up @@ -530,6 +530,25 @@ trait Implicits { self: Typer =>
case nil => acc
}

/** If the (result types of) the expected type, and both alternatives
* are all numeric value types, return the alternative which has
* the smaller numeric subtype as result type, if it exists.
* (This alternative is then discarded).
*/
def numericValueTieBreak(alt1: SearchSuccess, alt2: SearchSuccess): SearchResult = {
def isNumeric(tp: Type) = tp.typeSymbol.isNumericValueClass
def isProperSubType(tp1: Type, tp2: Type) =
tp1.isValueSubType(tp2) && !tp2.isValueSubType(tp1)
val rpt = pt.resultType
val rt1 = alt1.ref.widen.resultType
val rt2 = alt2.ref.widen.resultType
if (isNumeric(rpt) && isNumeric(rt1) && isNumeric(rt2))
if (isProperSubType(rt1, rt2)) alt1
else if (isProperSubType(rt2, rt1)) alt2
else NoImplicitMatches
else NoImplicitMatches
}

/** Convert a (possibly empty) list of search successes into a single search result */
def condense(hits: List[SearchSuccess]): SearchResult = hits match {
case best :: alts =>
Expand All @@ -539,7 +558,10 @@ trait Implicits { self: Typer =>
println(i"ambiguous refs: ${hits map (_.ref) map (_.show) mkString ", "}")
isAsGood(best.ref, alt.ref, explain = true)(ctx.fresh.withExploreTyperState)
*/
new AmbiguousImplicits(best.ref, alt.ref, pt, argument)
numericValueTieBreak(best, alt) match {
case eliminated: SearchSuccess => condense(hits.filter(_ ne eliminated))
case _ => new AmbiguousImplicits(best.ref, alt.ref, pt, argument)
}
case None =>
ctx.runInfo.useCount(best.ref) += 1
best
Expand Down
3 changes: 2 additions & 1 deletion src/dotty/tools/dotc/typer/ProtoTypes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ object ProtoTypes {
* 1. `tp` is a subtype of `pt`
* 2. `pt` is by name parameter type, and `tp` is compatible with its underlying type
* 3. there is an implicit conversion from `tp` to `pt`.
* 4. `tp` is a numeric subtype of `pt` (this case applies even if implicit conversions are disabled)
*/
def isCompatible(tp: Type, pt: Type)(implicit ctx: Context): Boolean =
tp.widenExpr <:< pt.widenExpr || viewExists(tp, pt)
(tp.widenExpr relaxed_<:< pt.widenExpr) || viewExists(tp, pt)

/** Test compatibility after normalization in a fresh typerstate. */
def normalizedCompatible(tp: Type, pt: Type)(implicit ctx: Context) = {
Expand Down
9 changes: 9 additions & 0 deletions tests/new/implicits.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
object Test {

class X(i: Int)

implicit def int2x(i: Int): X = new X(i)

val x: X = Byte.MinValue

}
4 changes: 4 additions & 0 deletions tests/pos/implicits1.scala
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ object Implicits {

val d: Int = z.foo("abc")

val x: X = Byte.MinValue

//import X.BarDeco

println(z.bar)
Expand All @@ -50,4 +52,6 @@ object Implicits {

val s: Modifier = Some("rd").getOrElse("")

val xx: Int = (1: Byte)

}
4 changes: 1 addition & 3 deletions tests/run/t8280.check
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
Int
Int
Int
Int
Int
Long
Int
Int
Int
Expand Down
12 changes: 9 additions & 3 deletions tests/run/t8280.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@ object Moop1 {
implicit object f1 extends (Int => String) { def apply(x: Int): String = "Int" }
implicit object f2 extends (Long => String) { def apply(x: Long): String = "Long" }

println(5: String)
// println(5: String)
// Dotty deviation. The above fails for Dotty with ambiguity error.
// Both f1 and f2 are applicable conversions for Int => String. Neither is better than the other.
// Scala2 contains a hack for backwards compatibility, which we are not forced to repeat.
// See discussion under SI-8280.

}
object ob2 {
implicit object f1 extends (Int => String) { def apply(x: Int): String = "Int" }
Expand All @@ -42,7 +47,7 @@ object Moop2 {
implicit def f1(x: Int): String = "Int"
implicit object f2 extends (Long => String) { def apply(x: Long): String = "Long" }

println(5: String)
println(5: String) // Dotty deviation: Dotty picks f2, because non-methods are more specific than methods
}
object ob2 {
implicit def f1(x: Int): String = "Int"
Expand All @@ -64,7 +69,8 @@ object Moop3 {
implicit val f1: Int => String = _ => "Int"
implicit object f2 extends (Long => String) { def apply(x: Long): String = "Long" }

println(5: String)
// println(5: String)
// Dotty deviation. This fails for Dotty with ambiguity error for similar reasons as ob1.
}
object ob2 {
implicit val f1: Int => String = _ => "Int"
Expand Down