Merge pull request #902 from dotty-staging/fix-numeric-implicit-args

Fix numeric implicit args

Author: odersky

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

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)
             }
@@ -532,6 +532,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 =>
@@ -541,7 +560,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

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) = {

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
+
+}

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)
+
 }

tests/run/t8280.check

@@ -1,8 +1,6 @@
 Int
 Int
-Int
-Int
-Int
+Long
 Int
 Int
 Int

tests/run/t8280.scala

@@ -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" }
@@ -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"
@@ -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"