From 3c7ce63a0a9b80a4ebac8315d47d324817fa31a4 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 6 Mar 2019 19:13:21 +0100 Subject: [PATCH 1/2] Tweak implicit resolution I have noted that the "more implicit parameters first" rule when applied unconditionally can lead to implicit search explosion. The test case is i3430.scala: println(Nil.min) // error: ambiguous We need to find an `Ordering[T]` for uninstantiated `T`. Before implicit arguments were taken into account we'd try two types (in this case Long and BigInteger) and get an ambiguity. Then we'd check whether any of the other candidates would dominate both of these types, which was not the case. So, we are done with an ambiguity. But when implicit arguments were taken into account there are many types that still are better then these. E.g. [T1: Ordering, T2: Ordering]: Ordering[(T1, T2)] and so on far all other supported arities of Ordering for tuples! So one of these has to be tried. That leads to searches for other uninstantiated orderings and so on. Running i3430.scala by hand, I got a compiler hang. I am not sure why the test suite succeeded nevertheless; there must be something surprising going on in the tests. My fix to get round these issue is that now implicit parameters are only considered if the result types of two alternatives are unifiable. Hopefully, that's not too constraining. --- compiler/src/dotty/tools/dotc/core/Types.scala | 3 +++ .../src/dotty/tools/dotc/printing/PlainPrinter.scala | 3 +-- .../src/dotty/tools/dotc/typer/Applications.scala | 11 +++++++---- .../reference/changed-features/implicit-resolution.md | 6 ++++-- tests/neg/i3430.check | 2 ++ tests/neg/i3430.scala | 2 +- 6 files changed, 18 insertions(+), 9 deletions(-) create mode 100644 tests/neg/i3430.check diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index e4490aa3c599..ed3b7f802ece 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -851,6 +851,9 @@ object Types { ctx.typeComparer.isSameType(this, that) } + final def frozen_=:=(that: Type)(implicit ctx: Context): Boolean = + ctx.typeComparer.isSameTypeWhenFrozen(this, that) + /** Is this type a primitive value type which can be widened to the primitive value type `that`? */ def isValueSubType(that: Type)(implicit ctx: Context): Boolean = widen match { case self: TypeRef if self.symbol.isPrimitiveValueClass => diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index a30ddea7af1c..39d5cbf4f7a8 100644 --- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -67,8 +67,7 @@ class PlainPrinter(_ctx: Context) extends Printer { else tp private def sameBound(lo: Type, hi: Type): Boolean = - try ctx.typeComparer.isSameTypeWhenFrozen(lo, hi) - catch { case NonFatal(ex) => false } + try lo frozen_=:= hi catch { case NonFatal(ex) => false } private def homogenizeArg(tp: Type) = tp match { case TypeBounds(lo, hi) if homogenizedView && sameBound(lo, hi) => homogenize(hi) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 6ae5ae5b04a7..9d90beec1d2e 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1370,11 +1370,14 @@ trait Applications extends Compatibility { self: Typer with Dynamic => val strippedType2 = stripImplicit(fullType2, +1) val result = compareWithTypes(strippedType1, strippedType2) - if (result != 0) result - else if (implicitBalance != 0) -implicitBalance.signum + if (result != 0 || !ctx.typerState.test(implicit ctx => strippedType1 =:= strippedType2)) + result + else if (implicitBalance != 0) + -implicitBalance.signum else if ((strippedType1 `ne` fullType1) || (strippedType2 `ne` fullType2)) compareWithTypes(fullType1, fullType2) - else 0 + else + 0 }} def narrowMostSpecific(alts: List[TermRef])(implicit ctx: Context): List[TermRef] = track("narrowMostSpecific") { @@ -1637,7 +1640,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic => // ps(i) = List(p_i_1, ..., p_i_n) -- i.e. a column // If all p_i_k's are the same, assume the type as formal parameter // type of the i'th parameter of the closure. - if (isUniform(ps)(ctx.typeComparer.isSameTypeWhenFrozen(_, _))) ps.head + if (isUniform(ps)(_ frozen_=:= _)) ps.head else WildcardType) def isPartial = // we should generate a partial function for the arg fn.get.isInstanceOf[untpd.Match] && diff --git a/docs/docs/reference/changed-features/implicit-resolution.md b/docs/docs/reference/changed-features/implicit-resolution.md index 927246f8ccc3..e905ed64de00 100644 --- a/docs/docs/reference/changed-features/implicit-resolution.md +++ b/docs/docs/reference/changed-features/implicit-resolution.md @@ -114,8 +114,10 @@ affect implicits on the language level. An alternative A is _more specific_ than an alternative B if - the relative weight of A over B is greater than the relative weight of B over A, or - - the relative weights are the same and A takes more inferable parameters than B, or - - the relative weights and the number of inferable parameters are the same and + - the relative weights are the same, and the returned types of A and B are + unifiable, and A takes more inferable parameters than B, or + - the relative weights and the number of inferable parameters are the same, and + the returned types of A and B are unifiable, and A is more specific than B if all inferable parameters in either alternative are replaced by regular parameters. diff --git a/tests/neg/i3430.check b/tests/neg/i3430.check new file mode 100644 index 000000000000..dc7717218b33 --- /dev/null +++ b/tests/neg/i3430.check @@ -0,0 +1,2 @@ +<44..44> in i3430.scala +ambiguous implicit arguments: both object Long in object Ordering and object BigDecimal in object Ordering match type Ordering[B] of parameter cmp of method min in trait TraversableOnce \ No newline at end of file diff --git a/tests/neg/i3430.scala b/tests/neg/i3430.scala index 5e88377dfdde..eaf3c50daa92 100644 --- a/tests/neg/i3430.scala +++ b/tests/neg/i3430.scala @@ -1,5 +1,5 @@ object Test extends App { - println(Nil.min) // error: no implicit found + println(Nil.min) // error: ambiguous } From d08de489b10fffa73055671d014ead72bbbae19e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 6 Mar 2019 20:45:20 +0100 Subject: [PATCH 2/2] Fix check file --- tests/neg/i3430.check | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/neg/i3430.check b/tests/neg/i3430.check index dc7717218b33..253c5a29fd2b 100644 --- a/tests/neg/i3430.check +++ b/tests/neg/i3430.check @@ -1,2 +1,2 @@ <44..44> in i3430.scala -ambiguous implicit arguments: both object Long in object Ordering and object BigDecimal in object Ordering match type Ordering[B] of parameter cmp of method min in trait TraversableOnce \ No newline at end of file +ambiguous implicit arguments: both object Long in object Ordering and object Short in object Ordering match type Ordering[B] of parameter cmp of method min in trait TraversableOnce \ No newline at end of file