From 68eb84e14528b0d39933e31c9f5215a8db51620d Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 31 Dec 2017 13:50:24 +0100 Subject: [PATCH 1/4] Avoid looping when inferring types with recursive lower bounds in constraints i3627.scala caused the typer to loop in several different ways. One was in isSubType, the other in wildApprox. The isSubType loop can be fixed by leaving monitoring on once the limit was reached for the remainder of the top-level subtype check. I am not exactly sure what caused the loop but the observation was a very long or infinite oscillation around the monitoring limit. Leaving monitoring on is certainly not incorrect - the previous trick to turn it off for recursions under the limit was purely for optimization. On the other hand I am not sure whether the new scheme is sufficient, or whether we can still have the situation of infinite recursions that stay under the depth limit. The wildApprox loop was a simple bug (the `seen` value was not propagated correctly to the TypeMap). The fix also removed a case in wildApproxBounds that had no clear purpose. --- compiler/src/dotty/tools/dotc/core/TypeComparer.scala | 10 ++++++---- compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala | 7 +++---- tests/neg/i3627.scala | 9 +++++++++ 3 files changed, 18 insertions(+), 8 deletions(-) create mode 100644 tests/neg/i3627.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 331a5adc06a9..d0537668f2b6 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -24,6 +24,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling { private[this] var pendingSubTypes: mutable.Set[(Type, Type)] = null private[this] var recCount = 0 + private[this] var monitored = false private[this] var needsGc = false @@ -101,9 +102,11 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling { if (tp2 eq NoType) return false if ((tp2 eq tp1) || (tp2 eq WildcardType)) return true try isSubType(tp1, tp2) - finally + finally { + monitored = false if (Config.checkConstraintsSatisfiable) assert(isSatisfiable, constraint.show) + } } protected def isSubType(tp1: Type, tp2: Type): Boolean = trace(s"isSubType ${traceInfo(tp1, tp2)}", subtyping) { @@ -114,9 +117,8 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling { val savedSuccessCount = successCount try { recCount = recCount + 1 - val result = - if (recCount < Config.LogPendingSubTypesThreshold) firstTry(tp1, tp2) - else monitoredIsSubType(tp1, tp2) + if (recCount >= Config.LogPendingSubTypesThreshold) monitored = true + val result = if (monitored) monitoredIsSubType(tp1, tp2) else firstTry(tp1, tp2) recCount = recCount - 1 if (!result) state.resetConstraintTo(saved) else if (recCount == 0 && needsGc) { diff --git a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala index 067b9c1e15df..44fdc89c87df 100644 --- a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala +++ b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala @@ -495,9 +495,7 @@ object ProtoTypes { tp.derivedTypeAlias(wildApprox(tp.alias, theMap, seen)) case tp @ TypeParamRef(poly, pnum) => def wildApproxBounds(bounds: TypeBounds) = - if (bounds.lo.isInstanceOf[NamedType] && bounds.hi.isInstanceOf[NamedType]) - WildcardType(wildApprox(bounds, theMap, seen).bounds) - else if (seen.contains(tp)) WildcardType + if (seen.contains(tp)) WildcardType else WildcardType(wildApprox(bounds, theMap, seen + tp).bounds) def unconstrainedApprox = wildApproxBounds(poly.paramInfos(pnum)) def approxPoly = @@ -544,7 +542,8 @@ object ProtoTypes { case _: ThisType | _: BoundType | NoPrefix => // default case, inlined for speed tp case _ => - (if (theMap != null) theMap else new WildApproxMap(seen)).mapOver(tp) + (if (theMap != null && seen.eq(theMap.seen)) theMap else new WildApproxMap(seen)) + .mapOver(tp) } @sharable object AssignProto extends UncachedGroundType with MatchAlways diff --git a/tests/neg/i3627.scala b/tests/neg/i3627.scala new file mode 100644 index 000000000000..f97d6e67d582 --- /dev/null +++ b/tests/neg/i3627.scala @@ -0,0 +1,9 @@ +trait Comparinator[T] { + def sort[T](x: Comparinator[_ >: T]) = () + sort((a: Int) => true) // error +} + +trait Comparinator2[T >: U, U] { + def sort[TT](x: Comparinator2[_ >: TT, U]) = () + sort((a: Int) => true) // error +} From 62dd6347ebe1684111bc39723a6a4717e059cb15 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 31 Dec 2017 13:54:56 +0100 Subject: [PATCH 2/4] Consider -Yexplain-lowlevel when showing goals of failed subtype assertions --- .../src/dotty/tools/dotc/core/TypeComparer.scala | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index d0537668f2b6..3dcd85da05f1 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -17,6 +17,8 @@ import reporting.trace /** Provides methods to compare types. */ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling { + import TypeComparer.show + implicit val ctx = initctx val state = ctx.typerState @@ -1573,7 +1575,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling { /** Show subtype goal that led to an assertion failure */ def showGoal(tp1: Type, tp2: Type)(implicit ctx: Context) = { - println(i"assertion failure for $tp1 <:< $tp2, frozen = $frozenConstraint") + println(i"assertion failure for ${show(tp1)} <:< ${show(tp2)}, frozen = $frozenConstraint") def explainPoly(tp: Type) = tp match { case tp: TypeParamRef => ctx.echo(s"TypeParamRef ${tp.show} found in ${tp.binder.show}") case tp: TypeRef if tp.symbol.exists => ctx.echo(s"typeref ${tp.show} found in ${tp.symbol.owner.show}") @@ -1605,6 +1607,11 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling { object TypeComparer { + private[core] def show(res: Any)(implicit ctx: Context) = res match { + case res: printing.Showable if !ctx.settings.YexplainLowlevel.value => res.show + case _ => String.valueOf(res) + } + /** Show trace of comparison operations when performing `op` as result string */ def explained[T](op: Context => T)(implicit ctx: Context): String = { val nestedCtx = ctx.fresh.setTypeComparerFn(new ExplainingTypeComparer(_)) @@ -1615,6 +1622,8 @@ object TypeComparer { /** A type comparer that can record traces of subtype operations */ class ExplainingTypeComparer(initctx: Context) extends TypeComparer(initctx) { + import TypeComparer.show + private[this] var indent = 0 private val b = new StringBuilder @@ -1631,11 +1640,6 @@ class ExplainingTypeComparer(initctx: Context) extends TypeComparer(initctx) { res } - private def show(res: Any) = res match { - case res: printing.Showable if !ctx.settings.YexplainLowlevel.value => res.show - case _ => String.valueOf(res) - } - override def isSubType(tp1: Type, tp2: Type) = traceIndented(s"${show(tp1)} <:< ${show(tp2)}${if (Config.verboseExplainSubtype) s" ${tp1.getClass} ${tp2.getClass}" else ""}${if (frozenConstraint) " frozen" else ""}") { super.isSubType(tp1, tp2) From 40bd71cd03db98871d25426a4fa9e2f9b436d74d Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 31 Dec 2017 13:55:38 +0100 Subject: [PATCH 3/4] Fail fast if LHS of subtype is Nothing This is purely an optimization. It would have fixed by itself the subtype loop in the first part of i3627.scala but not the one in the second part. --- compiler/src/dotty/tools/dotc/core/TypeComparer.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 3dcd85da05f1..13900832d3ab 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -288,6 +288,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling { if (isSubType(info1.alias, tp2)) return true if (tp1.prefix.isStable) return false case _ => + if (tp1 eq NothingType) return tp1 == tp2.bottomType } thirdTry(tp1, tp2) case tp1: TypeParamRef => From 56ec72042cde204b6922e3dd7d8b2221707d7be2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 31 Dec 2017 14:11:16 +0100 Subject: [PATCH 4/4] Fix test --- compiler/test/dotty/tools/dotc/CompilationTests.scala | 1 + tests/{neg => neg-custom-args}/i3627.scala | 0 2 files changed, 1 insertion(+) rename tests/{neg => neg-custom-args}/i3627.scala (100%) diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala index e79121c7fe6e..53e570095d02 100644 --- a/compiler/test/dotty/tools/dotc/CompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/CompilationTests.scala @@ -171,6 +171,7 @@ class CompilationTests extends ParallelTesting { compileFilesInDir("../tests/neg-tailcall", defaultOptions) + compileFilesInDir("../tests/neg-no-optimise", defaultOptions) + compileFile("../tests/neg-custom-args/i3246.scala", scala2Mode) + + compileFile("../tests/neg-custom-args/i3627.scala", allowDeepSubtypes) + compileFile("../tests/neg-custom-args/typers.scala", allowDoubleBindings) + compileFile("../tests/neg-custom-args/overrideClass.scala", scala2Mode) + compileFile("../tests/neg-custom-args/autoTuplingTest.scala", defaultOptions.and("-language:noAutoTupling")) + diff --git a/tests/neg/i3627.scala b/tests/neg-custom-args/i3627.scala similarity index 100% rename from tests/neg/i3627.scala rename to tests/neg-custom-args/i3627.scala