From 5ed617b9ac205b831ec69b782472b1afc5752378 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 1 Nov 2015 16:08:46 +0100 Subject: [PATCH 1/3] Better handling of implicits over numeric types. Compiling scala.math.BigDecimal and scala.math.BigInteger shows a problem. The conversion `int2bigInt` is not applicable to a Byte because `Byte -> Int` requires another implicit conversion. We fix that by using a new method relaxed_<:< for implicit compatibility checks, which always admits numeric widenings. This leads to another problem. Now the conversions implicit def byteToInt(x: Byte): Int implicit def byteToShort(x: Byte): Short are ambiguous when we try to convert from Byte to Int. We fix that by adding a "tie-break" to implicit search where if several methods match a numeric value result type and all have numeric value types as result types, we pick the numerically largest type that matches. --- src/dotty/tools/dotc/core/Types.scala | 5 +++- src/dotty/tools/dotc/typer/Implicits.scala | 30 ++++++++++++++++++--- src/dotty/tools/dotc/typer/ProtoTypes.scala | 3 ++- tests/new/implicits.scala | 9 +++++++ tests/pos/implicits1.scala | 4 +++ 5 files changed, 45 insertions(+), 6 deletions(-) create mode 100644 tests/new/implicits.scala diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala index 58a6d226d920..05fb963f9d6d 100644 --- a/src/dotty/tools/dotc/core/Types.scala +++ b/src/dotty/tools/dotc/core/Types.scala @@ -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 => @@ -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 diff --git a/src/dotty/tools/dotc/typer/Implicits.scala b/src/dotty/tools/dotc/typer/Implicits.scala index e3626fe20808..7dbf6d17c42d 100644 --- a/src/dotty/tools/dotc/typer/Implicits.scala +++ b/src/dotty/tools/dotc/typer/Implicits.scala @@ -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) } @@ -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 is + * the smaller numeric subtype, if it exists. (This alternative is then + * discarded). + */ + def tieBreak(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 => @@ -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) + tieBreak(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 diff --git a/src/dotty/tools/dotc/typer/ProtoTypes.scala b/src/dotty/tools/dotc/typer/ProtoTypes.scala index 40e919ee5f17..2b19d9db3712 100644 --- a/src/dotty/tools/dotc/typer/ProtoTypes.scala +++ b/src/dotty/tools/dotc/typer/ProtoTypes.scala @@ -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) = { diff --git a/tests/new/implicits.scala b/tests/new/implicits.scala new file mode 100644 index 000000000000..1a3e0b4da939 --- /dev/null +++ b/tests/new/implicits.scala @@ -0,0 +1,9 @@ +object Test { + + class X(i: Int) + + implicit def int2x(i: Int): X = new X(i) + + val x: X = Byte.MinValue + +} diff --git a/tests/pos/implicits1.scala b/tests/pos/implicits1.scala index d8ca76de5292..eda1346630d2 100644 --- a/tests/pos/implicits1.scala +++ b/tests/pos/implicits1.scala @@ -36,6 +36,8 @@ object Implicits { val d: Int = z.foo("abc") + val x: X = Byte.MinValue + //import X.BarDeco println(z.bar) @@ -50,4 +52,6 @@ object Implicits { val s: Modifier = Some("rd").getOrElse("") + val xx: Int = (1: Byte) + } From f48ea79df03fde590f0ba22dcc52f2c82227b9e9 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 1 Nov 2015 16:42:00 +0100 Subject: [PATCH 2/3] Disable failing test Dotty delivers an ambiguity error. The comment in the test argues why this is OK. --- src/dotty/tools/dotc/typer/Implicits.scala | 10 +++++----- tests/run/t8280.scala | 8 ++++++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/dotty/tools/dotc/typer/Implicits.scala b/src/dotty/tools/dotc/typer/Implicits.scala index 7dbf6d17c42d..54ecb24050e6 100644 --- a/src/dotty/tools/dotc/typer/Implicits.scala +++ b/src/dotty/tools/dotc/typer/Implicits.scala @@ -531,11 +531,11 @@ trait Implicits { self: Typer => } /** If the (result types of) the expected type, and both alternatives - * are all numeric value types, return the alternative which is - * the smaller numeric subtype, if it exists. (This alternative is then - * discarded). + * 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 tieBreak(alt1: SearchSuccess, alt2: SearchSuccess): SearchResult = { + 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) @@ -558,7 +558,7 @@ trait Implicits { self: Typer => println(i"ambiguous refs: ${hits map (_.ref) map (_.show) mkString ", "}") isAsGood(best.ref, alt.ref, explain = true)(ctx.fresh.withExploreTyperState) */ - tieBreak(best, alt) match { + numericValueTieBreak(best, alt) match { case eliminated: SearchSuccess => condense(hits.filter(_ ne eliminated)) case _ => new AmbiguousImplicits(best.ref, alt.ref, pt, argument) } diff --git a/tests/run/t8280.scala b/tests/run/t8280.scala index 0734d63b6ea4..1c9d5cbaa547 100644 --- a/tests/run/t8280.scala +++ b/tests/run/t8280.scala @@ -20,7 +20,11 @@ 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" } @@ -64,7 +68,7 @@ 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. } object ob2 { implicit val f1: Int => String = _ => "Int" From 34608562fa4f3697ddf4034fb802182a18c9f687 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 1 Nov 2015 17:24:36 +0100 Subject: [PATCH 3/3] Update check file and explain why it's different now. --- tests/run/t8280.check | 4 +--- tests/run/t8280.scala | 6 ++++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/run/t8280.check b/tests/run/t8280.check index ed392841c7ce..44c51f5aa0e9 100644 --- a/tests/run/t8280.check +++ b/tests/run/t8280.check @@ -1,8 +1,6 @@ Int Int -Int -Int -Int +Long Int Int Int diff --git a/tests/run/t8280.scala b/tests/run/t8280.scala index 1c9d5cbaa547..f433dcc32581 100644 --- a/tests/run/t8280.scala +++ b/tests/run/t8280.scala @@ -25,6 +25,7 @@ object Moop1 { // 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" } @@ -46,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" @@ -68,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) // Dotty deviation. This fails for Dotty with ambiguity error. + // 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"