Merge pull request #2065 from dotty-staging/change-implicit-conv2

Disallow subtypes of Function1 acting as implicit conversions

12 files changed, 81 insertions, 14 deletions

diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala
index 3cab75f93..1be47c1da 100644
--- a/compiler/src/dotty/tools/dotc/core/Definitions.scala
+++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala
@@ -299,6 +299,8 @@ class Definitions {
   lazy val ScalaPredefModuleRef = ctx.requiredModuleRef("scala.Predef")
   def ScalaPredefModule(implicit ctx: Context) = ScalaPredefModuleRef.symbol
 
+    lazy val Predef_ConformsR = ScalaPredefModule.requiredClass("$less$colon$less").typeRef
+    def Predef_Conforms(implicit ctx: Context) = Predef_ConformsR.symbol
     lazy val Predef_conformsR = ScalaPredefModule.requiredMethodRef("$conforms")
     def Predef_conforms(implicit ctx: Context) = Predef_conformsR.symbol
     lazy val Predef_classOfR = ScalaPredefModule.requiredMethodRef("classOf")
@@ -336,6 +338,8 @@ class Definitions {
   def DottyPredefModule(implicit ctx: Context) = DottyPredefModuleRef.symbol
 
     def Predef_eqAny(implicit ctx: Context) = DottyPredefModule.requiredMethod(nme.eqAny)
+    lazy val Predef_ImplicitConverterR = DottyPredefModule.requiredClass("ImplicitConverter").typeRef
+    def Predef_ImplicitConverter(implicit ctx: Context) = Predef_ImplicitConverterR.symbol
 
   lazy val DottyArraysModuleRef = ctx.requiredModuleRef("dotty.runtime.Arrays")
   def DottyArraysModule(implicit ctx: Context) = DottyArraysModuleRef.symbol
diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala
index 759cc62e9..ebbcbcc95 100644
--- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala
@@ -82,11 +82,29 @@ object Implicits {
           case tpw: TermRef =>
             false // can't discard overloaded refs
           case tpw =>
-            //if (ctx.typer.isApplicable(tp, argType :: Nil, resultType))
-            //  println(i"??? $tp is applicable to $this / typeSymbol = ${tpw.typeSymbol}")
-            !tpw.derivesFrom(defn.FunctionClass(1)) ||
-            ref.symbol == defn.Predef_conforms //
-              // as an implicit conversion, Predef.$conforms is a no-op, so exclude it
+            // Only direct instances of Function1 and direct or indirect instances of <:< are eligible as views.
+            // However, Predef.$conforms is not eligible, because it is a no-op.
+            //
+            // In principle, it would be cleanest if only implicit methods qualified
+            // as implicit conversions. We could achieve that by having standard conversions like
+            // this in Predef:
+            //
+            //    implicit def convertIfConforms[A, B](x: A)(implicit ev: A <:< B): B = ev(a)
+            //    implicit def convertIfConverter[A, B](x: A)(implicit ev: ImplicitConverter[A, B]): B = ev(a)
+            //
+            // (Once `<:<` inherits from `ImplicitConverter` we only need the 2nd one.)
+            // But clauses like this currently slow down implicit search a lot, because
+            // they are eligible for all pairs of types, and therefore are tried too often.
+            // We emulate instead these conversions directly in the search.
+            // The reason for leaving out `Predef_conforms` is that we know it adds
+            // nothing since it only relates subtype with supertype.
+            //
+            // We keep the old behavior under -language:Scala2.
+            val isFunctionInS2 = ctx.scala2Mode && tpw.derivesFrom(defn.FunctionClass(1))
+            val isImplicitConverter = tpw.derivesFrom(defn.Predef_ImplicitConverter)
+            val isConforms =
+              tpw.derivesFrom(defn.Predef_Conforms) && ref.symbol != defn.Predef_conforms
+            !(isFunctionInS2 || isImplicitConverter || isConforms)
         }
 
         def discardForValueType(tpw: Type): Boolean = tpw match {
diff --git a/library/src/dotty/DottyPredef.scala b/library/src/dotty/DottyPredef.scala
index 12040e0f3..e78fa9239 100644
--- a/library/src/dotty/DottyPredef.scala
+++ b/library/src/dotty/DottyPredef.scala
@@ -36,4 +36,24 @@ object DottyPredef {
   implicit def eqNumFloat : Eq[Number, Float]  = Eq
   implicit def eqDoubleNum: Eq[Double, Number] = Eq
   implicit def eqNumDouble: Eq[Number, Double] = Eq
+
+  /** A class for implicit values that can serve as implicit conversions
+   *  The implicit resolution algorithm will act as if there existed
+   *  the additional implicit definition:
+   *
+   *    def $implicitConversion[T, U](x: T)(c: ImplicitConverter[T, U]): U = c(x)
+   *
+   *  However, the presence of this definition would slow down implicit search since
+   *  its outermost type matches any pair of types. Therefore, implicit search
+   *  contains a special case in `Implicits#discardForView` which emulates the
+   *  conversion in a more efficient way.
+   *
+   *  Note that this is a SAM class - function literals are automatically converted
+   *  to `ImplicitConverter` values.
+   *
+   *  Also note that in bootstrapped dotty, `Predef.<:<` should inherit from
+   *  `ImplicitConverter`. This would cut the number of special cases in
+   *  `discardForView` from two to one.
+   */
+  abstract class ImplicitConverter[-T, +U] extends Function1[T, U]
 }
diff --git a/tests/neg/falseView.scala b/tests/neg/falseView.scala
new file mode 100644
index 000000000..613abe3f1
--- /dev/null
+++ b/tests/neg/falseView.scala
@@ -0,0 +1,7 @@
+object Test {
+
+  private implicit val xs: Map[String, Int] = ???
+
+  val x: Int = "abc" // error
+
+}
diff --git a/tests/pos/typerep-stephane.scala b/tests/pos-scala2/typerep-stephane.scala
index 2cb899591..2cb899591 100644
--- a/tests/pos/typerep-stephane.scala
+++ b/tests/pos-scala2/typerep-stephane.scala
diff --git a/tests/pos/viewtest1.scala b/tests/pos-scala2/viewtest1.scala
index 38945ad2f..38945ad2f 100644
--- a/tests/pos/viewtest1.scala
+++ b/tests/pos-scala2/viewtest1.scala
diff --git a/tests/pos/t0786.scala b/tests/pos/t0786.scala
index b320de0ed..9346afdff 100644
--- a/tests/pos/t0786.scala
+++ b/tests/pos/t0786.scala
@@ -15,7 +15,7 @@ object ImplicitProblem {
     def eval = f(nullval[T]).eval + 1
   }
 
-  def depth[T](n: T)(implicit ev: T => Rep[T]) = n.eval
+  def depth[T](n: T)(implicit ev: T => Rep[T]) = ev(n).eval
 
   def main(args: Array[String]): Unit = {
     println(depth(nullval[M[Int]]))  // (1) this works
diff --git a/tests/pos/t2421_delitedsl.scala b/tests/pos/t2421_delitedsl.scala
index 22f1ecd85..bde3593c9 100644
--- a/tests/pos/t2421_delitedsl.scala
+++ b/tests/pos/t2421_delitedsl.scala
@@ -1,6 +1,9 @@
 trait DeliteDSL {
   abstract class <~<[-From, +To] extends (From => To)
+
   implicit def trivial[A]: A <~< A = new (A <~< A) {def apply(x: A) = x}
+  implicit def convert_<-<[A, B](x: A)(implicit ev: A <~< B): B = ev(x)
+
 
   trait Forcible[T]
   object Forcible {
diff --git a/tests/run/iterator-from.scala b/tests/run/iterator-from.scala
index 4f403680c..c7c0f9809 100644
--- a/tests/run/iterator-from.scala
+++ b/tests/run/iterator-from.scala
@@ -11,7 +11,9 @@ object Test extends dotty.runtime.LegacyApp {
   val maxKey = 50
   val maxValue = 50
 
-  def testSet[A <% Ordered[A]](set: SortedSet[A], list: List[A]): Unit = {
+  implicit def convertIfView[A](x: A)(implicit view: A => Ordered[A]): Ordered[A] = view(x)
+
+  def testSet[A: Ordering](set: SortedSet[A], list: List[A]): Unit = {
     val distinctSorted = list.distinct.sorted
     assertEquals("Set size wasn't the same as list sze", set.size, distinctSorted.size)
 
@@ -24,7 +26,7 @@ object Test extends dotty.runtime.LegacyApp {
     }
   }
 
-  def testMap[A <% Ordered[A], B](map: SortedMap[A, B], list: List[(A, B)]): Unit = {
+  def testMap[A: Ordering, B](map: SortedMap[A, B], list: List[(A, B)]): Unit = {
     val distinctSorted = distinctByKey(list).sortBy(_._1)
     assertEquals("Map size wasn't the same as list sze", map.size, distinctSorted.size)
 
diff --git a/tests/run/puzzler54.scala b/tests/run/puzzler54.scala
new file mode 100644
index 000000000..9dd4cbb47
--- /dev/null
+++ b/tests/run/puzzler54.scala
@@ -0,0 +1,13 @@
+// Scala Puzzler 54
+object Test {
+  case class Card(number: Int, suit: String = "clubs") {
+    val value = (number % 13) + 1 // ace = 1, king = 13
+    def isInDeck(implicit deck: List[Card]) = deck contains this
+  }
+
+  def main(args: Array[String]) = {
+    implicit val deck = List(Card(1, "clubs"))
+    implicit def intToCard(n: Int): Card = Card(n)
+    assert(1.isInDeck)
+  }
+}
diff --git a/tests/run/t8280.check b/tests/run/t8280.check
index 44c51f5aa..b5885df48 100644
--- a/tests/run/t8280.check
+++ b/tests/run/t8280.check
@@ -1,7 +1,6 @@
-Int
-Int
 Long
 Int
 Int
 Int
 Int
+Int
diff --git a/tests/run/t8280.scala b/tests/run/t8280.scala
index f433dcc32..5fcbad0a3 100644
--- a/tests/run/t8280.scala
+++ b/tests/run/t8280.scala
@@ -37,7 +37,8 @@ object Moop1 {
     implicit object f1 extends (Int => String) { def apply(x: Int): String = "Int" }
     implicit val f2: Long => String = _ => "Long"
 
-    println(5: String)
+    //println(5: String)
+      // This picked f1 before, but is now disallowed since subtypes of functions are no longer implicit conversions
   }
 }
 
@@ -73,14 +74,14 @@ object Moop3 {
       // Dotty deviation. This fails for Dotty with ambiguity error for similar reasons as ob1.
   }
   object ob2 {
-    implicit val f1: Int => String  = _ => "Int"
+    implicit val f1: ImplicitConverter[Int, String]  = _ => "Int"
     implicit def f2(x: Long): String = "Long"
 
     println(5: String)
   }
   object ob3 {
-    implicit val f1: Int => String  = _ => "Int"
-    implicit val f2: Long => String = _ => "Long"
+    implicit val f1: ImplicitConverter[Int, String]  = _ => "Int"
+    implicit val f2: ImplicitConverter[Long, String] = _ => "Long"
 
     println(5: String)
   }