From bd04177b9dd2cf32f11df49558e36eb4eb158562 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 28 Jun 2018 18:49:12 +0200 Subject: [PATCH 1/7] Fix #876: Allow implicit conversions from singleton types I found a way to make this efficient (hopefully). --- .../src/dotty/tools/dotc/typer/Checking.scala | 10 ------ .../dotty/tools/dotc/typer/Implicits.scala | 31 +++++++++++++++++-- .../src/dotty/tools/dotc/typer/Typer.scala | 5 +-- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index e909906a8b7a..47893a9390c1 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -598,15 +598,6 @@ trait Checking { defn.ObjectType } - /** Check that a non-implicit parameter making up the first parameter section of an - * implicit conversion is not a singleton type. - */ - def checkImplicitParamsNotSingletons(vparamss: List[List[ValDef]])(implicit ctx: Context): Unit = vparamss match { - case (vparam :: Nil) :: _ if !(vparam.symbol is Implicit) => - checkNotSingleton(vparam.tpt, " to be parameter type of an implicit conversion") - case _ => - } - /** If `sym` is an implicit conversion, check that implicit conversions are enabled. * @pre sym.is(Implicit) */ @@ -948,7 +939,6 @@ trait NoChecking extends ReChecking { override def checkValue(tree: Tree, proto: Type)(implicit ctx: Context): tree.type = tree override def checkStable(tp: Type, pos: Position)(implicit ctx: Context): Unit = () override def checkClassType(tp: Type, pos: Position, traitReq: Boolean, stablePrefixReq: Boolean)(implicit ctx: Context): Type = tp - override def checkImplicitParamsNotSingletons(vparamss: List[List[ValDef]])(implicit ctx: Context): Unit = () override def checkImplicitConversionDefOK(sym: Symbol)(implicit ctx: Context): Unit = () override def checkImplicitConversionUseOK(sym: Symbol, pos: Position)(implicit ctx: Context): Unit = () override def checkFeasibleParent(tp: Type, pos: Position, where: => String = "")(implicit ctx: Context): Type = tp diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 31fd2d2026e5..185e236fcd5d 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -80,7 +80,8 @@ object Implicits { case mt: MethodType => mt.isImplicitMethod || mt.paramInfos.lengthCompare(1) != 0 || - !ctx.test(implicit ctx => argType relaxed_<:< mt.paramInfos.head) + !ctx.test(implicit ctx => + argType relaxed_<:< mt.paramInfos.head.widenSingleton) case poly: PolyType => // We do not need to call ProtoTypes#constrained on `poly` because // `refMatches` is always called with mode TypevarsMissContext enabled. @@ -88,7 +89,8 @@ object Implicits { case mt: MethodType => mt.isImplicitMethod || mt.paramInfos.length != 1 || - !ctx.test(implicit ctx => argType relaxed_<:< wildApprox(mt.paramInfos.head, null, Set.empty)) + !ctx.test(implicit ctx => + argType relaxed_<:< wildApprox(mt.paramInfos.head.widenSingleton, null, Set.empty)) case rtp => discardForView(wildApprox(rtp, null, Set.empty), argType) } @@ -132,6 +134,25 @@ object Implicits { case _ => false } + /** Widen singleton arguments of implicit conversions to their underlying type. + * This is necessary so that they can be found eligible for the argument type. + * Note that we always take the underlying type of a singleton type as the argument + * type, so that we get a reasonable implicit cache hit ratio. + */ + def adjustSingletonArg(tp: Type): Type = tp match { + case tp: PolyType => + val res = adjustSingletonArg(tp.resType) + if (res `eq` tp.resType) tp else tp.derivedLambdaType(resType = res) + case tp: MethodType => + tp.paramInfos match { + case (single: SingletonType) :: Nil => + tp.derivedLambdaType(paramInfos = single.widenSingleton :: Nil) + case _ => + tp + } + case _ => tp + } + (ref.symbol isAccessibleFrom ref.prefix) && { if (discard) { record("discarded eligible") @@ -139,7 +160,11 @@ object Implicits { } else { val ptNorm = normalize(pt, pt) // `pt` could be implicit function types, check i2749 - NoViewsAllowed.isCompatible(normalize(ref, pt), ptNorm) + val refAdjusted = + if (pt.isInstanceOf[ViewProto]) adjustSingletonArg(ref.widenSingleton) + else ref + val refNorm = normalize(refAdjusted, pt) + NoViewsAllowed.isCompatible(refNorm, ptNorm) } } } diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index fa48b6315a63..115c4f8130a8 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1413,10 +1413,7 @@ class Typer extends Namer val tparams1 = tparams mapconserve (typed(_).asInstanceOf[TypeDef]) val vparamss1 = vparamss nestedMapconserve (typed(_).asInstanceOf[ValDef]) vparamss1.foreach(checkNoForwardDependencies) - if (sym is Implicit) { - checkImplicitParamsNotSingletons(vparamss1) - checkImplicitConversionDefOK(sym) - } + if (sym is Implicit) checkImplicitConversionDefOK(sym) val tpt1 = checkSimpleKinded(typedType(tpt)) var rhsCtx = ctx From 4721da55453a33de0a3e4bacfb0ac0265bf4df10 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 28 Jun 2018 18:50:59 +0200 Subject: [PATCH 2/7] Add test case --- tests/run/i876.scala | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 tests/run/i876.scala diff --git a/tests/run/i876.scala b/tests/run/i876.scala new file mode 100644 index 000000000000..a791200ae7c5 --- /dev/null +++ b/tests/run/i876.scala @@ -0,0 +1,10 @@ +object Test extends App { + object O + implicit def foo(x: O.type): String = "hello" + val s: String = O + implicit def bar(x: s.type): Int = s.length + //implicit def bar2(x: String): Int = s.length + val l: Int = s + assert(s == "hello") + assert(l == 5) +} \ No newline at end of file From d3cd401cb0a27255d2d0dda61c849c2d7764537a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 28 Jun 2018 20:54:12 +0200 Subject: [PATCH 3/7] Fix neg test --- tests/neg/implicitDefs.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/neg/implicitDefs.scala b/tests/neg/implicitDefs.scala index 3bfe604341a3..a48d533d4d9f 100644 --- a/tests/neg/implicitDefs.scala +++ b/tests/neg/implicitDefs.scala @@ -7,7 +7,7 @@ object implicitDefs { implicit val x = 2 // error: type of implicit definition needs to be given explicitly implicit def y(x: Int) = 3 // error: result type of implicit definition needs to be given explicitly - implicit def z(a: x.type): String = "" // error: implicit conversion may not have a parameter of singleton type + implicit def z(a: x.type): String = "" // ok, used to be: implicit conversion may not have a parameter of singleton type def foo(implicit x: String) = 1 From 69bfb3413c7e23605ce43c8931a7f718c0020b62 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 28 Jun 2018 21:20:45 +0200 Subject: [PATCH 4/7] Fix #3781: Eligibility checks should ignore SingletonType When checking an implicit conversion for eligibility we should ignore any SingletonType upper bounds in its argument. This give rise to false negatives because we check against the underlying type when determining eligibility. --- .../src/dotty/tools/dotc/typer/Implicits.scala | 17 +++++++++-------- tests/run/i876.scala | 12 ++++++++++++ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 185e236fcd5d..c7f7dea3baae 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -76,12 +76,18 @@ object Implicits { def refMatches(ref: TermRef)(implicit ctx: Context) = /*trace(i"refMatches $ref $pt")*/ { + /** Widen type so that it is neither a singleton type nor it inherits from scala.Singleton. */ + def widenSingleton(tp: Type): Type = { + val wtp = tp.widenSingleton + if (wtp.derivesFrom(defn.SingletonClass)) defn.AnyType else wtp + } + def discardForView(tpw: Type, argType: Type): Boolean = tpw match { case mt: MethodType => mt.isImplicitMethod || mt.paramInfos.lengthCompare(1) != 0 || !ctx.test(implicit ctx => - argType relaxed_<:< mt.paramInfos.head.widenSingleton) + argType relaxed_<:< widenSingleton(mt.paramInfos.head)) case poly: PolyType => // We do not need to call ProtoTypes#constrained on `poly` because // `refMatches` is always called with mode TypevarsMissContext enabled. @@ -90,7 +96,7 @@ object Implicits { mt.isImplicitMethod || mt.paramInfos.length != 1 || !ctx.test(implicit ctx => - argType relaxed_<:< wildApprox(mt.paramInfos.head.widenSingleton, null, Set.empty)) + argType relaxed_<:< wildApprox(widenSingleton(mt.paramInfos.head), null, Set.empty)) case rtp => discardForView(wildApprox(rtp, null, Set.empty), argType) } @@ -144,12 +150,7 @@ object Implicits { val res = adjustSingletonArg(tp.resType) if (res `eq` tp.resType) tp else tp.derivedLambdaType(resType = res) case tp: MethodType => - tp.paramInfos match { - case (single: SingletonType) :: Nil => - tp.derivedLambdaType(paramInfos = single.widenSingleton :: Nil) - case _ => - tp - } + tp.derivedLambdaType(paramInfos = tp.paramInfos.map(widenSingleton)) case _ => tp } diff --git a/tests/run/i876.scala b/tests/run/i876.scala index a791200ae7c5..7480e3605e6f 100644 --- a/tests/run/i876.scala +++ b/tests/run/i876.scala @@ -7,4 +7,16 @@ object Test extends App { val l: Int = s assert(s == "hello") assert(l == 5) +} + +object Test3781 { + class Foo[T](val value : T) + object Foo { + implicit def fromXInt[T <: Int with Singleton](i : T): Foo[T] = new Foo[T](i) + } + class FooUser[T] { + def op[T2](that : Foo[T2]) : FooUser[T2] = new FooUser[T2] + } + val f = new FooUser[1] + val f2 = f op 2 } \ No newline at end of file From 5728b190adec7555c3ba7ed98cbb6d55d329ce36 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 29 Jun 2018 13:03:11 +0200 Subject: [PATCH 5/7] Some tweaks to improve performance --- compiler/src/dotty/tools/dotc/core/Mode.scala | 1 + .../src/dotty/tools/dotc/typer/Implicits.scala | 17 ++++++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Mode.scala b/compiler/src/dotty/tools/dotc/core/Mode.scala index 020b884b22b3..e689544503db 100644 --- a/compiler/src/dotty/tools/dotc/core/Mode.scala +++ b/compiler/src/dotty/tools/dotc/core/Mode.scala @@ -37,6 +37,7 @@ object Mode { * that TypeParamRefs can be sub- and supertypes of anything. See TypeComparer. */ val TypevarsMissContext = newMode(4, "TypevarsMissContext") + val CheckCyclic = newMode(5, "CheckCyclic") /** We are looking at the arguments of a supercall */ diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index c7f7dea3baae..858cad96936b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -71,17 +71,20 @@ object Implicits { /** The implicit references */ def refs: List[ImplicitRef] + private var SingletonClass: ClassSymbol = null + + /** Widen type so that it is neither a singleton type nor it inherits from scala.Singleton. */ + private def widenSingleton(tp: Type)(implicit ctx: Context): Type = { + if (SingletonClass == null) SingletonClass = defn.SingletonClass + val wtp = tp.widenSingleton + if (wtp.derivesFrom(SingletonClass)) defn.AnyType else wtp + } + /** Return those references in `refs` that are compatible with type `pt`. */ protected def filterMatching(pt: Type)(implicit ctx: Context): List[Candidate] = track("filterMatching") { def refMatches(ref: TermRef)(implicit ctx: Context) = /*trace(i"refMatches $ref $pt")*/ { - /** Widen type so that it is neither a singleton type nor it inherits from scala.Singleton. */ - def widenSingleton(tp: Type): Type = { - val wtp = tp.widenSingleton - if (wtp.derivesFrom(defn.SingletonClass)) defn.AnyType else wtp - } - def discardForView(tpw: Type, argType: Type): Boolean = tpw match { case mt: MethodType => mt.isImplicitMethod || @@ -150,7 +153,7 @@ object Implicits { val res = adjustSingletonArg(tp.resType) if (res `eq` tp.resType) tp else tp.derivedLambdaType(resType = res) case tp: MethodType => - tp.derivedLambdaType(paramInfos = tp.paramInfos.map(widenSingleton)) + tp.derivedLambdaType(paramInfos = tp.paramInfos.mapConserve(widenSingleton)) case _ => tp } From c8d9b0dc314c5d63756b011a90f038f5c0f347f6 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Fri, 29 Jun 2018 17:53:58 +0200 Subject: [PATCH 6/7] Slightly reformulate comment I may be wrong but I don't think that "nor it" is grammatically correct. --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 858cad96936b..848d928d7cf5 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -73,7 +73,7 @@ object Implicits { private var SingletonClass: ClassSymbol = null - /** Widen type so that it is neither a singleton type nor it inherits from scala.Singleton. */ + /** Widen type so that it is neither a singleton type nor a type that inherits from scala.Singleton. */ private def widenSingleton(tp: Type)(implicit ctx: Context): Type = { if (SingletonClass == null) SingletonClass = defn.SingletonClass val wtp = tp.widenSingleton From 673af1fd7b0b99bede6c70ccade4489657e82ddb Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Fri, 29 Jun 2018 18:04:42 +0200 Subject: [PATCH 7/7] Add neg test --- tests/neg/i876.scala | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 tests/neg/i876.scala diff --git a/tests/neg/i876.scala b/tests/neg/i876.scala new file mode 100644 index 000000000000..9affe2a5a607 --- /dev/null +++ b/tests/neg/i876.scala @@ -0,0 +1,8 @@ +object Test { + val a: Int = 1 + implicit def foo(x: a.type): String = "hi" + val b: Int = a + + val x: String = a // ok + val y: String = b // error: found Int, required String +}