From be418e06756a3b8ddc3f748008995155260eb7ae Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 31 May 2016 15:23:04 +0200 Subject: [PATCH 1/8] Document why we cannot cache all implicit scopes --- src/dotty/tools/dotc/typer/Implicits.scala | 13 ++++++++++++- tests/pos/implicit-scope-loop.scala | 17 +++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 tests/pos/implicit-scope-loop.scala diff --git a/src/dotty/tools/dotc/typer/Implicits.scala b/src/dotty/tools/dotc/typer/Implicits.scala index 38ac709bee5f..b3a1010cb511 100644 --- a/src/dotty/tools/dotc/typer/Implicits.scala +++ b/src/dotty/tools/dotc/typer/Implicits.scala @@ -363,7 +363,18 @@ trait ImplicitRunInfo { self: RunInfo => computeIScope(cacheResult = false) else implicitScopeCache get tp match { case Some(is) => is - case None => computeIScope(cacheResult = seen.isEmpty) + case None => + // Implicit scopes are tricky to cache because of loops. For example + // in `tests/pos/implicit-scope-loop.scala`, the scope of B contains + // the scope of A which contains the scope of B. We break the loop + // by returning EmptyTermRefSet in `collectCompanions` for types + // that we have already seen, but this means that we cannot cache + // the computed scope of A, it is incomplete. + // Keeping track of exactly where these loops happen would require a + // lot of book-keeping, instead we choose to be conservative and only + // cache scopes before any type has been seen. This is unfortunate + // because loops are very common for types in scala.collection. + computeIScope(cacheResult = seen.isEmpty) } } diff --git a/tests/pos/implicit-scope-loop.scala b/tests/pos/implicit-scope-loop.scala new file mode 100644 index 000000000000..162f1a52c053 --- /dev/null +++ b/tests/pos/implicit-scope-loop.scala @@ -0,0 +1,17 @@ +trait Dummy[T] + + +trait A[T] extends B +trait B extends Dummy[A[Int]] +object B { + implicit def theB: B = new B {} + implicit def theA: A[Int] = new A[Int] {} +} + +object Test { + def getB(implicit b: B) = b + def getA[T](implicit a: A[T]) = a + + getB + getA +} \ No newline at end of file From 818bf0b2aa3ec7544bb328aaad2b2bd75e724787 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 12 Jan 2016 22:12:03 +0100 Subject: [PATCH 2/8] Fix implicit scope caching bug The issue is subtle: the `tp` in scope in `def ofTypeImplicits` is the `tp` passed to the top-level `implicitScope` method, not the `tp` passed to the recursively called `iscope`, this means that before this commit, all intermediate `OfTypeImplicit` scopes cached while computing an implicit scope had their `tp` field incorrectly set, which means that we could miss implicits in later implicit searches. Note that the `implicit_cache.scala` test worked before this commit because of the restrictions on caching that exist since b8b0f381ef2cbcb7bad66fd3e7a9ae929baa45f6, it is included anyway because our caching strategy might change in the future. --- src/dotty/tools/dotc/typer/Implicits.scala | 11 ++++++----- tests/pos/implicit_cache.scala | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) create mode 100644 tests/pos/implicit_cache.scala diff --git a/src/dotty/tools/dotc/typer/Implicits.scala b/src/dotty/tools/dotc/typer/Implicits.scala index b3a1010cb511..44689d7588cc 100644 --- a/src/dotty/tools/dotc/typer/Implicits.scala +++ b/src/dotty/tools/dotc/typer/Implicits.scala @@ -338,8 +338,6 @@ trait ImplicitRunInfo { self: RunInfo => } } - def ofTypeImplicits(comps: TermRefSet) = new OfTypeImplicits(tp, comps)(ctx) - /** The implicit scope of type `tp` * @param isLifted Type `tp` is the result of a `liftToClasses` application */ @@ -349,9 +347,12 @@ trait ImplicitRunInfo { self: RunInfo => ctx.typerState.ephemeral = false try { val liftedTp = if (isLifted) tp else liftToClasses(tp) - val result = - if (liftedTp ne tp) iscope(liftedTp, isLifted = true) - else ofTypeImplicits(collectCompanions(tp)) + val refs = + if (liftedTp ne tp) + iscope(liftedTp, isLifted = true).companionRefs + else + collectCompanions(tp) + val result = new OfTypeImplicits(tp, refs)(ctx) if (ctx.typerState.ephemeral) record("ephemeral cache miss: implicitScope") else if (cacheResult) implicitScopeCache(tp) = result result diff --git a/tests/pos/implicit_cache.scala b/tests/pos/implicit_cache.scala new file mode 100644 index 000000000000..d124876e0274 --- /dev/null +++ b/tests/pos/implicit_cache.scala @@ -0,0 +1,16 @@ +class A +object A { + implicit def theA: A = new A +} +class Foo[T] +object Foo { + implicit def theFoo: Foo[A] = new Foo[A] +} + +object Test { + def getFooA(implicit foo: Foo[A]) = foo + def getA(implicit a: A) = a + + getFooA + getA +} From 088902cb6d86a5546f3f3aaf317ba78a51c0c604 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 12 Jan 2016 22:40:51 +0100 Subject: [PATCH 3/8] Do not miss implicits in type parameters of parents This did not work before because we incorrectly looked for their value in the prefix of the type instead of the type itself. --- src/dotty/tools/dotc/typer/Implicits.scala | 2 +- tests/pos/implicit_tparam.scala | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 tests/pos/implicit_tparam.scala diff --git a/src/dotty/tools/dotc/typer/Implicits.scala b/src/dotty/tools/dotc/typer/Implicits.scala index 44689d7588cc..6314ce8c9889 100644 --- a/src/dotty/tools/dotc/typer/Implicits.scala +++ b/src/dotty/tools/dotc/typer/Implicits.scala @@ -323,7 +323,7 @@ trait ImplicitRunInfo { self: RunInfo => def addParentScope(parent: TypeRef): Unit = { iscopeRefs(parent) foreach addRef for (param <- parent.typeParams) - comps ++= iscopeRefs(pre.member(param.name).info) + comps ++= iscopeRefs(tp.member(param.name).info) } val companion = cls.companionModule if (companion.exists) addRef(companion.valRef) diff --git a/tests/pos/implicit_tparam.scala b/tests/pos/implicit_tparam.scala new file mode 100644 index 000000000000..3b7cf911357a --- /dev/null +++ b/tests/pos/implicit_tparam.scala @@ -0,0 +1,12 @@ +class Foo[T] +class Bar extends Foo[A] + +class A +object A { + implicit val bar: Bar = new Bar +} + +object Test { + def getBar(implicit bar: Bar) = bar + getBar +} From 221e5d19da6e8cfbc6c0a1093c99b477a3592630 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 12 Jan 2016 23:50:28 +0100 Subject: [PATCH 4/8] Never include self types in named parts of a type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit According to SLS ยง 7.2, self types are not a named part of a type, so they're not part of the implicit scope. Before this commit, this was usually the case because we normally refer to a class using a TypeRef, but in some cases a class might appear as a ThisType, and ThisType#underlying returns the self type, we now use ThisType#tref instead which just returns a TypeRef corresponding to the class. --- src/dotty/tools/dotc/core/Types.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala index a318390d1803..42079b4b8238 100644 --- a/src/dotty/tools/dotc/core/Types.scala +++ b/src/dotty/tools/dotc/core/Types.scala @@ -3450,7 +3450,7 @@ object Types { case TypeBounds(_, hi) => apply(x, hi) case tp: ThisType => - apply(x, tp.underlying) + apply(x, tp.tref) case tp: ConstantType => apply(x, tp.underlying) case tp: MethodParam => From 5c2a19bfbbb158c809969fa2a9685b5c7e2695ea Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 12 Jan 2016 22:26:11 +0100 Subject: [PATCH 5/8] OfTypeImplicits: compute refs lazily Many intermediate `OfTypeImplicits` are created during a call to `implicitScope`, but they won't all be used so there is no need to compute `OfTypeImplicits#refs` unless it's actually used. --- src/dotty/tools/dotc/typer/Implicits.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/typer/Implicits.scala b/src/dotty/tools/dotc/typer/Implicits.scala index 6314ce8c9889..257828fb3bb3 100644 --- a/src/dotty/tools/dotc/typer/Implicits.scala +++ b/src/dotty/tools/dotc/typer/Implicits.scala @@ -106,7 +106,7 @@ object Implicits { */ class OfTypeImplicits(tp: Type, val companionRefs: TermRefSet)(initctx: Context) extends ImplicitRefs(initctx) { assert(initctx.typer != null) - val refs: List[TermRef] = { + lazy val refs: List[TermRef] = { val buf = new mutable.ListBuffer[TermRef] for (companion <- companionRefs) buf ++= companion.implicitMembers buf.toList From 8d0312f69966fbfcfc2f9602ccf660dfe7513885 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 12 Jan 2016 23:00:57 +0100 Subject: [PATCH 6/8] Avoid creating AndTypes with Any This reduces the number of implicit scopes we cache. --- src/dotty/tools/dotc/core/Types.scala | 7 ++++++- src/dotty/tools/dotc/typer/Implicits.scala | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala index 42079b4b8238..f514a329e1b9 100644 --- a/src/dotty/tools/dotc/core/Types.scala +++ b/src/dotty/tools/dotc/core/Types.scala @@ -2145,7 +2145,12 @@ object Types { unique(new CachedAndType(tp1, tp2)) } def make(tp1: Type, tp2: Type)(implicit ctx: Context): Type = - if (tp1 eq tp2) tp1 else apply(tp1, tp2) + if ((tp1 eq tp2) || (tp2 eq defn.AnyType)) + tp1 + else if (tp1 eq defn.AnyType) + tp2 + else + apply(tp1, tp2) } abstract case class OrType(tp1: Type, tp2: Type) extends CachedGroundType with AndOrType { diff --git a/src/dotty/tools/dotc/typer/Implicits.scala b/src/dotty/tools/dotc/typer/Implicits.scala index 257828fb3bb3..b59d177b168c 100644 --- a/src/dotty/tools/dotc/typer/Implicits.scala +++ b/src/dotty/tools/dotc/typer/Implicits.scala @@ -287,7 +287,7 @@ trait ImplicitRunInfo { self: RunInfo => case tp: TypeRef if tp.symbol.isAbstractOrAliasType => val pre = tp.prefix def joinClass(tp: Type, cls: ClassSymbol) = - AndType(tp, cls.typeRef.asSeenFrom(pre, cls.owner)) + AndType.make(tp, cls.typeRef.asSeenFrom(pre, cls.owner)) val lead = if (tp.prefix eq NoPrefix) defn.AnyType else apply(tp.prefix) (lead /: tp.classSymbols)(joinClass) case tp: TypeVar => From 044d29dd6b133bfb28468366ee92046b77adf6f9 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 31 May 2016 15:43:01 +0200 Subject: [PATCH 7/8] Don't compute implicit scopes for synthetic Lambda traits --- src/dotty/tools/dotc/typer/Implicits.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/typer/Implicits.scala b/src/dotty/tools/dotc/typer/Implicits.scala index b59d177b168c..7de40294dfcb 100644 --- a/src/dotty/tools/dotc/typer/Implicits.scala +++ b/src/dotty/tools/dotc/typer/Implicits.scala @@ -284,10 +284,13 @@ trait ImplicitRunInfo { self: RunInfo => override implicit protected val ctx: Context = liftingCtx override def stopAtStatic = true def apply(tp: Type) = tp match { + case tp: TypeRef if tp.symbol.isLambdaTrait => + defn.AnyType case tp: TypeRef if tp.symbol.isAbstractOrAliasType => val pre = tp.prefix def joinClass(tp: Type, cls: ClassSymbol) = - AndType.make(tp, cls.typeRef.asSeenFrom(pre, cls.owner)) + if (cls.isLambdaTrait) tp + else AndType.make(tp, cls.typeRef.asSeenFrom(pre, cls.owner)) val lead = if (tp.prefix eq NoPrefix) defn.AnyType else apply(tp.prefix) (lead /: tp.classSymbols)(joinClass) case tp: TypeVar => From 295c981f719d09a690076af9d87d48679b1ceb42 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 26 May 2016 05:35:38 +0200 Subject: [PATCH 8/8] Overloading resolution: prefer directly applicable methods If directly applicable alternatives exists, do not try other alternatives. The original motivation for this change was to reduce the number of searches for implicit views we do since some overloaded methods like `Int#+` are used a lot, but it turns out that this also makes more code compile (see `overload_directly_applicable.scala` for an example), this change does not seem to match what the specification says (it does not define a notion of "directly applicable") but it does match the behavior of scalac, and it seems useful in general. --- src/dotty/tools/dotc/typer/Applications.scala | 14 ++++++++++---- tests/run/overload_directly_applicable.check | 1 + tests/run/overload_directly_applicable.scala | 16 ++++++++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 tests/run/overload_directly_applicable.check create mode 100644 tests/run/overload_directly_applicable.scala diff --git a/src/dotty/tools/dotc/typer/Applications.scala b/src/dotty/tools/dotc/typer/Applications.scala index 2b0cc4033509..f743c5784ea5 100644 --- a/src/dotty/tools/dotc/typer/Applications.scala +++ b/src/dotty/tools/dotc/typer/Applications.scala @@ -1146,11 +1146,17 @@ trait Applications extends Compatibility { self: Typer => alts } - def narrowByTrees(alts: List[TermRef], args: List[Tree], resultType: Type): List[TermRef] = - alts filter ( alt => - if (!ctx.isAfterTyper) isApplicable(alt, targs, args, resultType) - else isDirectlyApplicable(alt, targs, args, resultType) + def narrowByTrees(alts: List[TermRef], args: List[Tree], resultType: Type): List[TermRef] = { + val alts2 = alts.filter(alt => + isDirectlyApplicable(alt, targs, args, resultType) ) + if (alts2.isEmpty && !ctx.isAfterTyper) + alts.filter(alt => + isApplicable(alt, targs, args, resultType) + ) + else + alts2 + } val alts1 = narrowBySize(alts) //ctx.log(i"narrowed by size: ${alts1.map(_.symbol.showDcl)}%, %") diff --git a/tests/run/overload_directly_applicable.check b/tests/run/overload_directly_applicable.check new file mode 100644 index 000000000000..e2cf5e790565 --- /dev/null +++ b/tests/run/overload_directly_applicable.check @@ -0,0 +1 @@ +C1 diff --git a/tests/run/overload_directly_applicable.scala b/tests/run/overload_directly_applicable.scala new file mode 100644 index 000000000000..d204d424a056 --- /dev/null +++ b/tests/run/overload_directly_applicable.scala @@ -0,0 +1,16 @@ +class A +class B + +class C1 { + def f(x: A): Unit = println("C1") +} +class C2 extends C1 { + def f(x: B): Unit = println("C2") +} + +object Test extends C2 { + implicit def a2b(x: A): B = new B + def main(args: Array[String]): Unit = { + f(new A) + } +}