Skip to content

Disallow subtypes of Function1 acting as implicit conversions #2065

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 5 commits into from
Mar 9, 2017
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
4 changes: 4 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down
28 changes: 23 additions & 5 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
20 changes: 20 additions & 0 deletions library/src/dotty/DottyPredef.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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]
}
7 changes: 7 additions & 0 deletions tests/neg/falseView.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
object Test {

private implicit val xs: Map[String, Int] = ???

val x: Int = "abc" // error

}
File renamed without changes.
2 changes: 1 addition & 1 deletion tests/pos/t0786.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions tests/pos/t2421_delitedsl.scala
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
6 changes: 4 additions & 2 deletions tests/run/iterator-from.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)

Expand Down
13 changes: 13 additions & 0 deletions tests/run/puzzler54.scala
Original file line number Diff line number Diff line change
@@ -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)
}
}
3 changes: 1 addition & 2 deletions tests/run/t8280.check
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
Int
Int
Long
Int
Int
Int
Int
Int
9 changes: 5 additions & 4 deletions tests/run/t8280.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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)
}
Expand Down