From 4be1409a0443987eb187f4527312851a91947db5 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 25 Apr 2018 19:28:56 +0200 Subject: [PATCH 01/21] Detect and flag deep findMember recursions Deep findMember recursions can happen as a recult of illegal cyclic structures. I don't think there's a complete way to avoid these structures before the cyclic search can happen. So we now detect it by imposing a recursion limit (configurable, by default 100). --- compiler/src/dotty/tools/dotc/config/Config.scala | 5 ----- .../src/dotty/tools/dotc/config/ScalaSettings.scala | 3 ++- compiler/src/dotty/tools/dotc/core/TypeOps.scala | 6 ------ compiler/src/dotty/tools/dotc/core/Types.scala | 12 +++++++----- compiler/src/dotty/tools/dotc/typer/Inliner.scala | 4 ++-- 5 files changed, 11 insertions(+), 19 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala index 8abbb81bae85..ea96d9dcd5d9 100644 --- a/compiler/src/dotty/tools/dotc/config/Config.scala +++ b/compiler/src/dotty/tools/dotc/config/Config.scala @@ -184,11 +184,6 @@ object Config { */ final val LogPendingFindMemberThreshold = 9 - /** Maximal number of outstanding recursive calls to findMember before backing out - * when findMemberLimit is set. - */ - final val PendingFindMemberLimit = LogPendingFindMemberThreshold * 4 - /** When in IDE, turn StaleSymbol errors into warnings instead of crashing */ final val ignoreStaleInIDE = true } diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index bbbfb88262f4..d28ee0c9b73c 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -63,7 +63,7 @@ class ScalaSettings extends Settings.SettingGroup { val Xhelp = BooleanSetting("-X", "Print a synopsis of advanced options.") val XnoForwarders = BooleanSetting("-Xno-forwarders", "Do not generate static forwarders in mirror classes.") val XminImplicitSearchDepth = IntSetting("-Xmin-implicit-search-depth", "Set number of levels of implicit searches undertaken before checking for divergence.", 5) - val xmaxInlines = IntSetting("-Xmax-inlines", "Maximal number of successive inlines", 32) + val XmaxInlines = IntSetting("-Xmax-inlines", "Maximal number of successive inlines", 32) val XmaxClassfileName = IntSetting("-Xmax-classfile-name", "Maximum filename length for generated classes", 255, 72 to 255) val Xmigration = VersionSetting("-Xmigration", "Warn about constructs whose behavior may have changed since version.") val Xprint = PhasesSetting("-Xprint", "Print out program after") @@ -74,6 +74,7 @@ class ScalaSettings extends Settings.SettingGroup { val XmainClass = StringSetting("-Xmain-class", "path", "Class for manifest's Main-Class entry (only useful with -d )", "") val XnoValueClasses = BooleanSetting("-Xno-value-classes", "Do not use value classes. Helps debugging.") val XreplLineWidth = IntSetting("-Xrepl-line-width", "Maximal number of columns per line for REPL output", 390) + val XmaxMemberRecursions = IntSetting("-Xmax-member-recursions", "Maximal number of recursive calls to find-member", 100) val XfatalWarnings = BooleanSetting("-Xfatal-warnings", "Fail the compilation if there are any warnings.") val XverifySignatures = BooleanSetting("-Xverify-signatures", "Verify generic signatures in generated bytecode.") diff --git a/compiler/src/dotty/tools/dotc/core/TypeOps.scala b/compiler/src/dotty/tools/dotc/core/TypeOps.scala index 1ff312dae475..05fd66b54282 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeOps.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeOps.scala @@ -326,10 +326,4 @@ trait TypeOps { this: Context => // TODO: Make standalone object. object TypeOps { @sharable var track = false // !!!DEBUG - - /** When a property with this key is set in a context, it limits the number - * of recursive member searches. If the limit is reached, findMember returns - * NoDenotation. - */ - val findMemberLimit = new Property.Key[Unit] } diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 79e4a469b00a..67eb93b096e6 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -666,17 +666,15 @@ object Types { val recCount = ctx.findMemberCount if (recCount >= Config.LogPendingFindMemberThreshold) { - if (ctx.property(TypeOps.findMemberLimit).isDefined && - ctx.findMemberCount > Config.PendingFindMemberLimit) - return NoDenotation + if (ctx.findMemberCount > ctx.settings.XmaxMemberRecursions.value) + throw new CyclicFindMember(pre, name) ctx.pendingMemberSearches = name :: ctx.pendingMemberSearches } ctx.findMemberCount = recCount + 1 - //assert(ctx.findMemberCount < 20) try go(this) catch { case ex: Throwable => - core.println(i"findMember exception for $this member $name, pre = $pre") + core.println(s"findMember exception for $this member $name, pre = $pre, recCount = $recCount") throw ex // DEBUG } finally { @@ -4581,6 +4579,10 @@ object Types { if (ctx.debug) printStackTrace() } + class CyclicFindMember(pre: Type, name: Name)(implicit ctx: Context) extends TypeError( + i"""member search with prefix $pre too deep. + |searches, from inner to outer: .${ctx.pendingMemberSearches}% .%""") + private def otherReason(pre: Type)(implicit ctx: Context): String = pre match { case pre: ThisType if pre.cls.givenSelfType.exists => i"\nor the self type of $pre might not contain all transitive dependencies" diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index eac354893f06..267ac4cadf22 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -122,13 +122,13 @@ object Inliner { * and body that replace it. */ def inlineCall(tree: Tree, pt: Type)(implicit ctx: Context): Tree = - if (enclosingInlineds.length < ctx.settings.xmaxInlines.value) { + if (enclosingInlineds.length < ctx.settings.XmaxInlines.value) { val body = bodyToInline(tree.symbol) // can typecheck the tree and thereby produce errors if (ctx.reporter.hasErrors) tree else new Inliner(tree, body).inlined(pt) } else errorTree( tree, - i"""|Maximal number of successive inlines (${ctx.settings.xmaxInlines.value}) exceeded, + i"""|Maximal number of successive inlines (${ctx.settings.XmaxInlines.value}) exceeded, |Maybe this is caused by a recursive inline method? |You can use -Xmax:inlines to change the limit.""" ) From 961cae938728c827c9fd11afe0e8a9278346e33b Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 26 Apr 2018 13:24:57 +0200 Subject: [PATCH 02/21] Add test for cyclic references involving members of inherited and intersected types Test every template and every type intersection for cycles in its members. The tests are done as early possible without causing spurious cycles, which for intersections means PostTyper, unfortunately. Still missing: Do the test also for members reachable in common value definitions. --- .../src/dotty/tools/dotc/core/Types.scala | 9 +++- .../tools/dotc/transform/PostTyper.scala | 7 ++- .../src/dotty/tools/dotc/typer/Checking.scala | 35 ++++++++++++ .../src/dotty/tools/dotc/typer/Typer.scala | 2 + tests/run/returning.scala | 53 +++++++++++++++++++ 5 files changed, 104 insertions(+), 2 deletions(-) create mode 100644 tests/run/returning.scala diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 67eb93b096e6..b17f02b386d7 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -4579,8 +4579,15 @@ object Types { if (ctx.debug) printStackTrace() } + def showPrefixSafely(pre: Type)(implicit ctx: Context): String = pre.stripTypeVar match { + case pre: TermRef => i"${pre.termSymbol.name}." + case pre: TypeRef => i"${pre.typeSymbol.name}#" + case pre: TypeProxy => showPrefixSafely(pre.underlying) + case _ => if (pre.typeSymbol.exists) i"${pre.typeSymbol.name}#" else "." + } + class CyclicFindMember(pre: Type, name: Name)(implicit ctx: Context) extends TypeError( - i"""member search with prefix $pre too deep. + i"""member search for ${showPrefixSafely(pre)}$name too deep. |searches, from inner to outer: .${ctx.pendingMemberSearches}% .%""") private def otherReason(pre: Type)(implicit ctx: Context): String = pre match { diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala index ad4885f14669..2ad44d9cb3da 100644 --- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala @@ -6,7 +6,7 @@ import scala.collection.mutable import core._ import typer.Checking import Types._, Contexts._, Names._, Flags._, DenotTransformers._ -import SymDenotations._, StdNames._, Annotations._, Trees._ +import SymDenotations._, StdNames._, Annotations._, Trees._, Scopes._ import Decorators._ import Symbols._, SymUtils._ import reporting.diagnostic.messages.{ImportRenamedTwice, NotAMember, SuperCallsNotAllowedInline} @@ -283,6 +283,11 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase case tpe => tpe } ) + case tree: AndTypeTree => + // Ideally, this should be done by Typer, but we run into cyclic references + // when trying to typecheck self types which are intersections. + Checking.checkNonCyclicInherited(tree.tpe, tree.left.tpe :: tree.right.tpe :: Nil, EmptyScope, tree.pos) + super.transform(tree) case Import(expr, selectors) => val exprTpe = expr.tpe val seen = mutable.Set.empty[Name] diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 7c5d0894a18a..898e54f22e22 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -13,6 +13,7 @@ import Symbols._ import Trees._ import TreeInfo._ import ProtoTypes._ +import Scopes._ import CheckRealizable._ import ErrorReporting.errorTree @@ -321,6 +322,36 @@ object Checking { checkTree((), refinement) } + /** Check type members inherited from different `parents` of `joint` type for cycles, + * unless a type with the same name aleadry appears in `decls`. + * @return true iff no cycles were detected + */ + def checkNonCyclicInherited(joint: Type, parents: List[Type], decls: Scope, pos: Position)(implicit ctx: Context): Unit = { + def qualifies(sym: Symbol) = sym.name.isTypeName && !sym.is(Private) + val abstractTypeNames = + for (parent <- parents; mbr <- parent.abstractTypeMembers if qualifies(mbr.symbol)) + yield mbr.name.asTypeName + + for (name <- abstractTypeNames) + try { + val mbr = joint.member(name) + mbr.info match { + case bounds: TypeBounds => + val res = checkNonCyclic(mbr.symbol, bounds, reportErrors = true).isError + if (res) + println(i"cyclic ${mbr.symbol}, $bounds -> $res") + res + case _ => + false + } + } + catch { + case ex: CyclicFindMember => + ctx.error(em"cyclic reference involving type $name", pos) + true + } + } + /** Check that symbol's definition is well-formed. */ def checkWellFormed(sym: Symbol)(implicit ctx: Context): Unit = { def fail(msg: Message) = ctx.error(msg, sym.pos) @@ -520,6 +551,9 @@ trait Checking { def checkNonCyclic(sym: Symbol, info: TypeBounds, reportErrors: Boolean)(implicit ctx: Context): Type = Checking.checkNonCyclic(sym, info, reportErrors) + def checkNonCyclicInherited(joint: Type, parents: List[Type], decls: Scope, pos: Position)(implicit ctx: Context): Unit = + Checking.checkNonCyclicInherited(joint, parents, decls, pos) + /** Check that Java statics and packages can only be used in selections. */ def checkValue(tree: Tree, proto: Type)(implicit ctx: Context): tree.type = { @@ -906,6 +940,7 @@ trait ReChecking extends Checking { trait NoChecking extends ReChecking { import tpd._ override def checkNonCyclic(sym: Symbol, info: TypeBounds, reportErrors: Boolean)(implicit ctx: Context): Type = info + override def checkNonCyclicInherited(joint: Type, parents: List[Type], decls: Scope, pos: Position)(implicit ctx: Context): Unit = () 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 diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index f9c7fdaeef7b..2045237a5933 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1546,6 +1546,8 @@ class Typer extends Namer cls, isRequired, cdef.pos) } + checkNonCyclicInherited(cls.thisType, cls.classParents, cls.info.decls, cdef.pos) + // check value class constraints checkDerivedValueClass(cls, body1) diff --git a/tests/run/returning.scala b/tests/run/returning.scala new file mode 100644 index 000000000000..f0af05e2ba14 --- /dev/null +++ b/tests/run/returning.scala @@ -0,0 +1,53 @@ +package scala.util.control { + +object NonLocalReturns { + + class ReturnThrowable[T] extends ControlThrowable { + private var myResult: T = _ + def throwReturn(result: T): Nothing = { + myResult = result + throw this + } + def result: T = myResult + } + + def throwReturn[T](result: T)(implicit returner: ReturnThrowable[T]): Nothing = + returner.throwReturn(result) + + def returning[T](op: implicit ReturnThrowable[T] => T): T = { + val returner = new ReturnThrowable[T] + try op(returner) + catch { + case ex: ReturnThrowable[_] => + if (ex `eq` returner) ex.result.asInstanceOf[T] else throw ex + } + } +} +} + +object Test extends App { + + import scala.util.control.NonLocalReturns._ + import scala.collection.mutable.ListBuffer + + def has(xs: List[Int], elem: Int) = + returning { + for (x <- xs) + if (x == elem) throwReturn(true) + false + } + + def takeUntil(xs: List[Int], elem: Int) = + returning { + var buf = new ListBuffer[Int] + for (x <- xs) + yield { + if (x == elem) throwReturn(buf.toList) + buf += x + x + } + } + + assert(has(List(1, 2, 3), 2)) + assert(takeUntil(List(1, 2, 3), 3) == List(1, 2)) +} \ No newline at end of file From 8a4f24e7d67a964cb32a77f654bc789548bc8497 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 26 Apr 2018 13:36:00 +0200 Subject: [PATCH 03/21] Add test --- tests/neg/i4368.scala | 150 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 tests/neg/i4368.scala diff --git a/tests/neg/i4368.scala b/tests/neg/i4368.scala new file mode 100644 index 000000000000..34a245c3df67 --- /dev/null +++ b/tests/neg/i4368.scala @@ -0,0 +1,150 @@ +object Test1 { + trait X { + type A = B + type B + } + trait Y { + type A + type B = A + } + trait Z extends X with Y // error: cyclic +} + +object Test2 { + trait W { + type A + type B + } + trait X { z: W => + type A = z.B + type B + } + trait Y { z: W => + type A + type B = z.A + } + trait Z extends X with Y // error: cyclic +} + +object Test3 { + trait W { + type A + type B + } + trait X { z: W => + type A = z.B + type B + } + trait Y { z: W => + type A + type B = z.A + } + + object App { + type Z = X with Y + val z: Z = z + val a: z.A = a // error: too deep + } +} + +object Test4 { + trait X[F[_]] { + protected type A = F[B] + protected type B + } + trait Y[F[_]] { + protected type A + protected type B = F[A] + } + + trait Fix[F[_]] extends X[F] with Y[F] { + type Result = A // error: too deep + } +} + +object Test5 { + trait X { + type A = B + type B + } + trait Y { + type A + type B = A + } + + object App { + type Z = X & Y + val z: Z = z + val a: z.A = a // error: too deep + } +} + +object Test6 { + trait W { type T <: W; val t: T } + trait X { + type A = b.T + val a : A = b.t + type B <: W + val b : B + } + trait Y { + type A <: W + val a : A + type B = a.T + val b = a.t + } + trait Z extends X with Y // error: cyclic +} + +object Test7 { + class Fix[F[_]] { + class Foo { type R >: F[T] <: F[T] } + type T = F[Foo#R] + } + + object App { + type Nat = Fix[Option]#T // error: too deep + } +} +/* +object Test8 { + + class A { + type T = B#U // error: cyclic + } + + class B { + type U = A#T + } +} +*/ +object Test9 { + trait W { + type A + } + trait X extends W { + type A = B + type B + } + trait Y extends W { + type A + type B = A + } + + trait Foo[X <: W, Y <: W] { + type Z = X & Y + val z: Z + val a: z.A + } + + trait Boo { + val f: Foo[X, Y] + } + + trait Baz extends Boo { + val a = f.a // error: member search too deep + // this should be a cyclic error, but it goes undetected + // scalac reports a volatility error, but the dotty equivalent (checkRealizable) + // is checked too late. + } +} \ No newline at end of file From 04bec4b5298f94403923890aae7f75b1918db4ac Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 26 Apr 2018 13:37:39 +0200 Subject: [PATCH 04/21] Detect more cyclic references Detect more cyclic references be generalizing the condition which prefixes are interesting. --- compiler/src/dotty/tools/dotc/typer/Checking.scala | 7 ++++++- tests/neg/i4368.scala | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 898e54f22e22..c7f018bac020 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -222,7 +222,11 @@ object Checking { // global symbols when doing the cyclicity check. def isInteresting(prefix: Type): Boolean = prefix.stripTypeVar match { case NoPrefix => true - case prefix: ThisType => sym.owner.isClass && prefix.cls.isContainedIn(sym.owner) + case prefix: ThisType => + sym.owner.isClass && ( + prefix.cls.isContainedIn(sym.owner) // sym reachable through outer references + || sym.owner.isContainedIn(prefix.cls) // sym reachable through member references + ) case prefix: NamedType => (!sym.is(Private) && prefix.derivesFrom(sym.owner)) || (!prefix.symbol.isStaticOwner && isInteresting(prefix.prefix)) @@ -232,6 +236,7 @@ object Checking { case _: RefinedOrRecType | _: AppliedType => true case _ => false } + if (isInteresting(pre)) { val pre1 = this(pre, false, false) if (locked.contains(tp) || tp.symbol.infoOrCompleter.isInstanceOf[NoCompleter]) diff --git a/tests/neg/i4368.scala b/tests/neg/i4368.scala index 34a245c3df67..ce1bc25fdeca 100644 --- a/tests/neg/i4368.scala +++ b/tests/neg/i4368.scala @@ -98,12 +98,12 @@ object Test6 { object Test7 { class Fix[F[_]] { - class Foo { type R >: F[T] <: F[T] } + class Foo { type R >: F[T] <: F[T] } // error: cyclic type T = F[Foo#R] } object App { - type Nat = Fix[Option]#T // error: too deep + type Nat = Fix[Option]#T } } /* From 8240d4445870b72fd35da194f5b5d0f785121269 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 26 Apr 2018 16:53:20 +0200 Subject: [PATCH 05/21] Rearrange TypeError exceptions - Move them all to TypeErrors.scala - Systematically use `toMessage` for reporting - Catch stack overflows instead of counting recursions --- .../tools/dotc/config/ScalaSettings.scala | 1 - .../src/dotty/tools/dotc/core/Types.scala | 61 +------------------ .../src/dotty/tools/dotc/sbt/ExtractAPI.scala | 5 +- .../dotc/transform/OverridingPairs.scala | 5 +- .../src/dotty/tools/dotc/typer/Checking.scala | 11 ++-- .../tools/dotc/typer/ErrorReporting.scala | 20 ------ .../dotty/tools/dotc/typer/RefChecks.scala | 2 +- .../src/dotty/tools/dotc/typer/Typer.scala | 3 +- tests/neg/i4368.scala | 2 +- 9 files changed, 14 insertions(+), 96 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index d28ee0c9b73c..1328a6d5d812 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -74,7 +74,6 @@ class ScalaSettings extends Settings.SettingGroup { val XmainClass = StringSetting("-Xmain-class", "path", "Class for manifest's Main-Class entry (only useful with -d )", "") val XnoValueClasses = BooleanSetting("-Xno-value-classes", "Do not use value classes. Helps debugging.") val XreplLineWidth = IntSetting("-Xrepl-line-width", "Maximal number of columns per line for REPL output", 390) - val XmaxMemberRecursions = IntSetting("-Xmax-member-recursions", "Maximal number of recursive calls to find-member", 100) val XfatalWarnings = BooleanSetting("-Xfatal-warnings", "Fail the compilation if there are any warnings.") val XverifySignatures = BooleanSetting("-Xverify-signatures", "Verify generic signatures in generated bytecode.") diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index b17f02b386d7..d65c843bb18e 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -20,7 +20,6 @@ import util.Positions.{Position, NoPosition} import util.Stats._ import util.{DotClass, SimpleIdentitySet} import reporting.diagnostic.Message -import reporting.diagnostic.messages.CyclicReferenceInvolving import ast.tpd._ import ast.TreeTypeMap import printing.Texts._ @@ -32,10 +31,9 @@ import Uniques._ import collection.{mutable, Seq} import config.Config import annotation.tailrec -import Flags.FlagSet import language.implicitConversions import scala.util.hashing.{ MurmurHash3 => hashing } -import config.Printers.{core, typr, cyclicErrors} +import config.Printers.{core, typr} import java.lang.ref.WeakReference object Types { @@ -665,17 +663,14 @@ object Types { } val recCount = ctx.findMemberCount - if (recCount >= Config.LogPendingFindMemberThreshold) { - if (ctx.findMemberCount > ctx.settings.XmaxMemberRecursions.value) - throw new CyclicFindMember(pre, name) + if (recCount >= Config.LogPendingFindMemberThreshold) ctx.pendingMemberSearches = name :: ctx.pendingMemberSearches - } ctx.findMemberCount = recCount + 1 try go(this) catch { case ex: Throwable => core.println(s"findMember exception for $this member $name, pre = $pre, recCount = $recCount") - throw ex // DEBUG + throw new RecursionOverflow("find-member", pre, name, ex) } finally { if (recCount >= Config.LogPendingFindMemberThreshold) @@ -4565,56 +4560,6 @@ object Types { def apply(pre: Type, name: Name)(implicit ctx: Context): Boolean = true } - // ----- Exceptions ------------------------------------------------------------- - - class TypeError(msg: String) extends Exception(msg) - - class MalformedType(pre: Type, denot: Denotation, absMembers: Set[Name]) - extends TypeError( - s"malformed type: $pre is not a legal prefix for $denot because it contains abstract type member${if (absMembers.size == 1) "" else "s"} ${absMembers.mkString(", ")}") - - class MissingType(pre: Type, name: Name)(implicit ctx: Context) extends TypeError( - i"""cannot resolve reference to type $pre.$name - |the classfile defining the type might be missing from the classpath${otherReason(pre)}""") { - if (ctx.debug) printStackTrace() - } - - def showPrefixSafely(pre: Type)(implicit ctx: Context): String = pre.stripTypeVar match { - case pre: TermRef => i"${pre.termSymbol.name}." - case pre: TypeRef => i"${pre.typeSymbol.name}#" - case pre: TypeProxy => showPrefixSafely(pre.underlying) - case _ => if (pre.typeSymbol.exists) i"${pre.typeSymbol.name}#" else "." - } - - class CyclicFindMember(pre: Type, name: Name)(implicit ctx: Context) extends TypeError( - i"""member search for ${showPrefixSafely(pre)}$name too deep. - |searches, from inner to outer: .${ctx.pendingMemberSearches}% .%""") - - private def otherReason(pre: Type)(implicit ctx: Context): String = pre match { - case pre: ThisType if pre.cls.givenSelfType.exists => - i"\nor the self type of $pre might not contain all transitive dependencies" - case _ => "" - } - - class CyclicReference private (val denot: SymDenotation) - extends TypeError(s"cyclic reference involving $denot") { - def toMessage(implicit ctx: Context) = CyclicReferenceInvolving(denot) - } - - object CyclicReference { - def apply(denot: SymDenotation)(implicit ctx: Context): CyclicReference = { - val ex = new CyclicReference(denot) - if (!(ctx.mode is Mode.CheckCyclic)) { - cyclicErrors.println(ex.getMessage) - for (elem <- ex.getStackTrace take 200) - cyclicErrors.println(elem.toString) - } - ex - } - } - - class MergeError(msg: String, val tp1: Type, val tp2: Type) extends TypeError(msg) - // ----- Debug --------------------------------------------------------- @sharable var debugTrace = false diff --git a/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala b/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala index 3bcf99e8b3cc..e26fe556510b 100644 --- a/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala +++ b/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala @@ -14,7 +14,6 @@ import Symbols._ import NameOps._ import NameKinds.DefaultGetterName import typer.Inliner -import typer.ErrorReporting.cyclicErrorMsg import transform.ValueClasses import transform.SymUtils._ import dotty.tools.io.File @@ -243,10 +242,10 @@ private class ExtractAPICollector(implicit val ctx: Context) extends ThunkHolder val ancestorTypes0 = try linearizedAncestorTypes(cinfo) catch { - case ex: CyclicReference => + case ex: TypeError => // See neg/i1750a for an example where a cyclic error can arise. // The root cause in this example is an illegal "override" of an inner trait - ctx.error(cyclicErrorMsg(ex), csym.pos) + ctx.error(ex.toMessage, csym.pos) defn.ObjectType :: Nil } if (ValueClasses.isDerivedValueClass(csym)) { diff --git a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala index 57858600dad6..2f166891f353 100644 --- a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala +++ b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala @@ -6,7 +6,6 @@ import Flags._, Symbols._, Contexts._, Types._, Scopes._, Decorators._ import util.HashSet import collection.mutable import collection.immutable.BitSet -import typer.ErrorReporting.cyclicErrorMsg import scala.annotation.tailrec /** A module that can produce a kind of iterator (`Cursor`), @@ -133,10 +132,10 @@ object OverridingPairs { } } catch { - case ex: CyclicReference => + case ex: TypeError => // See neg/i1750a for an example where a cyclic error can arise. // The root cause in this example is an illegal "override" of an inner trait - ctx.error(cyclicErrorMsg(ex), base.pos) + ctx.error(ex.toMessage, base.pos) } } else { curEntry = curEntry.prev diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index c7f018bac020..37d45fd3d45d 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -342,18 +342,15 @@ object Checking { val mbr = joint.member(name) mbr.info match { case bounds: TypeBounds => - val res = checkNonCyclic(mbr.symbol, bounds, reportErrors = true).isError - if (res) - println(i"cyclic ${mbr.symbol}, $bounds -> $res") - res + !checkNonCyclic(mbr.symbol, bounds, reportErrors = true).isError case _ => - false + true } } catch { - case ex: CyclicFindMember => + case ex: RecursionOverflow => ctx.error(em"cyclic reference involving type $name", pos) - true + false } } diff --git a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala index 6e03384e31e1..79799c9013a1 100644 --- a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala +++ b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala @@ -23,26 +23,6 @@ object ErrorReporting { ErrorType(msg) } - def cyclicErrorMsg(ex: CyclicReference)(implicit ctx: Context) = { - val cycleSym = ex.denot.symbol - def errorMsg(msg: Message, cx: Context): Message = - if (cx.mode is Mode.InferringReturnType) { - cx.tree match { - case tree: untpd.DefDef if !tree.tpt.typeOpt.exists => - OverloadedOrRecursiveMethodNeedsResultType(tree.name) - case tree: untpd.ValDef if !tree.tpt.typeOpt.exists => - RecursiveValueNeedsResultType(tree.name) - case _ => - errorMsg(msg, cx.outer) - } - } else msg - - if (cycleSym.is(Implicit, butNot = Method) && cycleSym.owner.isTerm) - CyclicReferenceInvolvingImplicit(cycleSym) - else - errorMsg(ex.toMessage, ctx) - } - def wrongNumberOfTypeArgs(fntpe: Type, expectedArgs: List[ParamInfo], actual: List[untpd.Tree], pos: Position)(implicit ctx: Context) = errorType(WrongNumberOfTypeArgs(fntpe, expectedArgs, actual)(ctx), pos) diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 628c069d99d5..af7dbdaed390 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -1614,7 +1614,7 @@ class RefChecks extends MiniPhase { thisPhase => } catch { case ex: TypeError => if (settings.debug) ex.printStackTrace() - unit.error(tree.pos, ex.getMessage()) + unit.error(tree.pos, ex.toMssage) tree } finally { localTyper = savedLocalTyper diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 2045237a5933..289542c54817 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1862,8 +1862,7 @@ class Typer extends Namer assertPositioned(tree) try adapt(typedUnadapted(tree, pt, locked), pt, locked) catch { - case ex: CyclicReference => errorTree(tree, cyclicErrorMsg(ex)) - case ex: TypeError => errorTree(tree, ex.getMessage) + case ex: TypeError => errorTree(tree, ex.toMessage) } } diff --git a/tests/neg/i4368.scala b/tests/neg/i4368.scala index ce1bc25fdeca..5ffe099811a1 100644 --- a/tests/neg/i4368.scala +++ b/tests/neg/i4368.scala @@ -110,7 +110,7 @@ object Test7 { object Test8 { class A { - type T = B#U // error: cyclic + type T = B#U } class B { From e6af77076b7ef27c2d91c104c7f1e77c859a195d Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 26 Apr 2018 19:00:00 +0200 Subject: [PATCH 06/21] Further refactoring of stackoverflow handling This now handles all errors from #4368 to #4372 and also #318. --- .../tools/dotc/core/TypeApplications.scala | 5 +- .../dotty/tools/dotc/core/TypeComparer.scala | 9 +- .../dotty/tools/dotc/core/TypeErrors.scala | 123 ++++++++++++++++++ .../src/dotty/tools/dotc/core/Types.scala | 10 +- .../tools/dotc/typer/VarianceChecker.scala | 34 ++--- .../dotty/tools/dotc/CompilationTests.scala | 1 + tests/neg-custom-args/i4372.scala | 3 + tests/neg/i4368.scala | 29 ++++- 8 files changed, 192 insertions(+), 22 deletions(-) create mode 100644 compiler/src/dotty/tools/dotc/core/TypeErrors.scala create mode 100644 tests/neg-custom-args/i4372.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeApplications.scala b/compiler/src/dotty/tools/dotc/core/TypeApplications.scala index 83188406d6a1..75732475ac73 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeApplications.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeApplications.scala @@ -168,7 +168,7 @@ class TypeApplications(val self: Type) extends AnyVal { * any type parameter that is-rebound by the refinement. */ final def typeParams(implicit ctx: Context): List[TypeParamInfo] = /*>|>*/ track("typeParams") /*<|<*/ { - self match { + try self match { case self: TypeRef => val tsym = self.symbol if (tsym.isClass) tsym.typeParams @@ -193,6 +193,9 @@ class TypeApplications(val self: Type) extends AnyVal { case _ => Nil } + catch { + case ex: Throwable => handleRecursive("type parameters of", self.show, ex) + } } /** If `self` is a higher-kinded type, its type parameters, otherwise Nil */ diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index d2e122b7bfe8..52c39ab473cc 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -121,7 +121,11 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling { protected def isSubType(tp1: Type, tp2: Type, a: ApproxState): Boolean = { val saved = approx this.approx = a - try recur(tp1, tp2) finally this.approx = saved + try recur(tp1, tp2) + catch { + case ex: Throwable => handleRecursive("subtype", i"$tp1 <:< $tp2", ex, weight = 2) + } + finally this.approx = saved } protected def isSubType(tp1: Type, tp2: Type): Boolean = isSubType(tp1, tp2, NoApprox) @@ -161,7 +165,8 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling { try { pendingSubTypes += p firstTry - } finally { + } + finally { pendingSubTypes -= p } } diff --git a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala new file mode 100644 index 000000000000..e15688dea314 --- /dev/null +++ b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala @@ -0,0 +1,123 @@ +package dotty.tools +package dotc +package core + +import util.common._ +import Types._ +import Symbols._ +import Flags._ +import Names._ +import Contexts._ +import SymDenotations._ +import Denotations._ +import Decorators._ +import reporting.diagnostic.Message +import reporting.diagnostic.messages._ +import ast.untpd +import config.Printers.cyclicErrors + +class TypeError(msg: String) extends Exception(msg) { + def this() = this("") + def toMessage(implicit ctx: Context): Message = getMessage +} + +class MalformedType(pre: Type, denot: Denotation, absMembers: Set[Name]) extends TypeError { + override def toMessage(implicit ctx: Context): Message = + i"malformed type: $pre is not a legal prefix for $denot because it contains abstract type member${if (absMembers.size == 1) "" else "s"} ${absMembers.mkString(", ")}" +} + +class MissingType(pre: Type, name: Name) extends TypeError { + private def otherReason(pre: Type)(implicit ctx: Context): String = pre match { + case pre: ThisType if pre.cls.givenSelfType.exists => + i"\nor the self type of $pre might not contain all transitive dependencies" + case _ => "" + } + + override def toMessage(implicit ctx: Context): Message = { + if (ctx.debug) printStackTrace() + i"""cannot resolve reference to type $pre.$name + |the classfile defining the type might be missing from the classpath${otherReason(pre)}""" + } +} + +class RecursionOverflow(val op: String, details: => String, previous: Throwable, val weight: Int) extends TypeError { + + def explanation = s"$op $details" + + private def recursions: List[RecursionOverflow] = { + val nested = previous match { + case previous: RecursionOverflow => previous.recursions + case _ => Nil + } + this :: nested + } + + def opsString(rs: List[RecursionOverflow])(implicit ctx: Context): String = { + val maxShown = 20 + if (rs.lengthCompare(maxShown) > 0) + i"""${opsString(rs.take(maxShown / 2))} + | ... + |${opsString(rs.takeRight(maxShown / 2))}""" + else + (rs.map(_.explanation): List[String]).mkString("\n ", "\n| ", "") + } + + override def toMessage(implicit ctx: Context): Message = { + val mostCommon = recursions.groupBy(_.op).toList.maxBy(_._2.map(_.weight).sum)._2.reverse + s"""Recursion limit exceeded. + |Maybe there is an illegal cyclic reference? + |A recurring operation is (inner to outer): + |${opsString(mostCommon)}""".stripMargin + } + + override def fillInStackTrace(): Throwable = this + override def getStackTrace() = previous.getStackTrace() +} + +object handleRecursive { + def apply(op: String, details: => String, exc: Throwable, weight: Int = 1): Nothing = exc match { + case _: RecursionOverflow => + throw new RecursionOverflow(op, details, exc, weight) + case _ => + var e = exc + while (e != null && !e.isInstanceOf[StackOverflowError]) e = e.getCause + if (e != null) throw new RecursionOverflow(op, details, e, weight) + else throw exc + } +} + +class CyclicReference private (val denot: SymDenotation) extends TypeError { + override def toMessage(implicit ctx: Context) = { + val cycleSym = denot.symbol + def errorMsg(msg: Message, cx: Context): Message = + if (cx.mode is Mode.InferringReturnType) { + cx.tree match { + case tree: untpd.DefDef if !tree.tpt.typeOpt.exists => + OverloadedOrRecursiveMethodNeedsResultType(tree.name) + case tree: untpd.ValDef if !tree.tpt.typeOpt.exists => + RecursiveValueNeedsResultType(tree.name) + case _ => + errorMsg(msg, cx.outer) + } + } else msg + + if (cycleSym.is(Implicit, butNot = Method) && cycleSym.owner.isTerm) + CyclicReferenceInvolvingImplicit(cycleSym) + else + CyclicReferenceInvolving(denot) + } +} + +object CyclicReference { + def apply(denot: SymDenotation)(implicit ctx: Context): CyclicReference = { + val ex = new CyclicReference(denot) + if (!(ctx.mode is Mode.CheckCyclic)) { + cyclicErrors.println(ex.getMessage) + for (elem <- ex.getStackTrace take 200) + cyclicErrors.println(elem.toString) + } + ex + } +} + +class MergeError(msg: String, val tp1: Type, val tp2: Type) extends TypeError(msg) \ No newline at end of file diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index d65c843bb18e..eea4dd2b51ae 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -670,7 +670,15 @@ object Types { catch { case ex: Throwable => core.println(s"findMember exception for $this member $name, pre = $pre, recCount = $recCount") - throw new RecursionOverflow("find-member", pre, name, ex) + + def showPrefixSafely(pre: Type)(implicit ctx: Context): String = pre.stripTypeVar match { + case pre: TermRef => i"${pre.termSymbol.name}." + case pre: TypeRef => i"${pre.typeSymbol.name}#" + case pre: TypeProxy => showPrefixSafely(pre.underlying) + case _ => if (pre.typeSymbol.exists) i"${pre.typeSymbol.name}#" else "." + } + + handleRecursive("find-member", i"${showPrefixSafely(pre)}$name", ex) } finally { if (recCount >= Config.LogPendingFindMemberThreshold) diff --git a/compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala b/compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala index 0ba36c2ac306..ee98321dcc78 100644 --- a/compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala +++ b/compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala @@ -82,21 +82,25 @@ class VarianceChecker()(implicit ctx: Context) { * same is true of the parameters (ValDefs). */ def apply(status: Option[VarianceError], tp: Type): Option[VarianceError] = trace(s"variance checking $tp of $base at $variance", variances) { - if (status.isDefined) status - else tp match { - case tp: TypeRef => - val sym = tp.symbol - if (sym.variance != 0 && base.isContainedIn(sym.owner)) checkVarianceOfSymbol(sym) - else if (sym.isAliasType) this(status, sym.info.bounds.hi) - else foldOver(status, tp) - case tp: MethodOrPoly => - this(status, tp.resultType) // params will be checked in their TypeDef or ValDef nodes. - case AnnotatedType(_, annot) if annot.symbol == defn.UncheckedVarianceAnnot => - status - //case tp: ClassInfo => - // ??? not clear what to do here yet. presumably, it's all checked at local typedefs - case _ => - foldOver(status, tp) + try + if (status.isDefined) status + else tp match { + case tp: TypeRef => + val sym = tp.symbol + if (sym.variance != 0 && base.isContainedIn(sym.owner)) checkVarianceOfSymbol(sym) + else if (sym.isAliasType) this(status, sym.info.bounds.hi) + else foldOver(status, tp) + case tp: MethodOrPoly => + this(status, tp.resultType) // params will be checked in their TypeDef or ValDef nodes. + case AnnotatedType(_, annot) if annot.symbol == defn.UncheckedVarianceAnnot => + status + //case tp: ClassInfo => + // ??? not clear what to do here yet. presumably, it's all checked at local typedefs + case _ => + foldOver(status, tp) + } + catch { + case ex: Throwable => handleRecursive("variance check of", tp.show, ex) } } diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala index aaa8ac216b5b..03906caae63d 100644 --- a/compiler/test/dotty/tools/dotc/CompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/CompilationTests.scala @@ -187,6 +187,7 @@ class CompilationTests extends ParallelTesting { compileFile("tests/neg-custom-args/noimports.scala", defaultOptions.and("-Yno-imports")) + compileFile("tests/neg-custom-args/noimports2.scala", defaultOptions.and("-Yno-imports")) + compileFile("tests/neg-custom-args/i3882.scala", allowDeepSubtypes) + + compileFile("tests/neg-custom-args/i4372.scala", allowDeepSubtypes) + compileFile("tests/neg-custom-args/i1754.scala", allowDeepSubtypes) + compileFilesInDir("tests/neg-custom-args/isInstanceOf", allowDeepSubtypes and "-Xfatal-warnings") + compileFile("tests/neg-custom-args/i3627.scala", allowDeepSubtypes) diff --git a/tests/neg-custom-args/i4372.scala b/tests/neg-custom-args/i4372.scala new file mode 100644 index 000000000000..ebe4b1192d6b --- /dev/null +++ b/tests/neg-custom-args/i4372.scala @@ -0,0 +1,3 @@ +object i4372 { + class X[A >: X[_ <: X[_]] <: X[A]] +} diff --git a/tests/neg/i4368.scala b/tests/neg/i4368.scala index 5ffe099811a1..c5c0f5223d95 100644 --- a/tests/neg/i4368.scala +++ b/tests/neg/i4368.scala @@ -106,18 +106,16 @@ object Test7 { type Nat = Fix[Option]#T } } -/* object Test8 { class A { - type T = B#U + type T = B#U // error: cyclic } class B { type U = A#T } } -*/ object Test9 { trait W { type A @@ -147,4 +145,29 @@ object Test9 { // scalac reports a volatility error, but the dotty equivalent (checkRealizable) // is checked too late. } +} +object i4369 { + trait X { self => + type R <: Z + type Z >: X { type R = self.R; type Z = self.R } + } + class Foo extends X { type R = Foo; type Z = Foo } // error: too deep +} +object i4370 { + class Foo { type R = A } // error: cyclic + type A = List[Foo#R] +} +object i4371 { + class Foo { type A = Boo#B } // error: cyclic + class Boo { type B = Foo#A } +} +object i318 { + trait Y { + type A <: { type T >: B } + type B >: { type T >: A } + } + + val y: Y = ??? + val a: y.A = ??? // error: too deep + val b: y.B = a // error: too deep } \ No newline at end of file From 01b5ecfcd8017b3a117fffd7e87afcdaa9c9662d Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 26 Apr 2018 19:42:16 +0200 Subject: [PATCH 07/21] Fix CyclicError reporting --- .../dotty/tools/dotc/core/TypeErrors.scala | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala index e15688dea314..ebb3c7da9687 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala @@ -86,25 +86,27 @@ object handleRecursive { } } -class CyclicReference private (val denot: SymDenotation) extends TypeError { +class CyclicReference private (val denot: SymDenotation)(implicit val ctx: Context) extends TypeError { + + def errorMsg(cx: Context): Message = + if (cx.mode is Mode.InferringReturnType) { + cx.tree match { + case tree: untpd.DefDef if !tree.tpt.typeOpt.exists => + OverloadedOrRecursiveMethodNeedsResultType(tree.name) + case tree: untpd.ValDef if !tree.tpt.typeOpt.exists => + RecursiveValueNeedsResultType(tree.name) + case _ => + errorMsg(cx.outer) + } + } + else CyclicReferenceInvolving(denot) + override def toMessage(implicit ctx: Context) = { val cycleSym = denot.symbol - def errorMsg(msg: Message, cx: Context): Message = - if (cx.mode is Mode.InferringReturnType) { - cx.tree match { - case tree: untpd.DefDef if !tree.tpt.typeOpt.exists => - OverloadedOrRecursiveMethodNeedsResultType(tree.name) - case tree: untpd.ValDef if !tree.tpt.typeOpt.exists => - RecursiveValueNeedsResultType(tree.name) - case _ => - errorMsg(msg, cx.outer) - } - } else msg - if (cycleSym.is(Implicit, butNot = Method) && cycleSym.owner.isTerm) CyclicReferenceInvolvingImplicit(cycleSym) else - CyclicReferenceInvolving(denot) + errorMsg(this.ctx) } } From 25c6abb654d5cc5fc50a450116d99d7bd3921c29 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 26 Apr 2018 20:58:51 +0200 Subject: [PATCH 08/21] Fix context for cyclic error message --- compiler/src/dotty/tools/dotc/core/TypeErrors.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala index ebb3c7da9687..edb8e374a597 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala @@ -86,7 +86,7 @@ object handleRecursive { } } -class CyclicReference private (val denot: SymDenotation)(implicit val ctx: Context) extends TypeError { +class CyclicReference private (val denot: SymDenotation) extends TypeError { def errorMsg(cx: Context): Message = if (cx.mode is Mode.InferringReturnType) { @@ -106,7 +106,7 @@ class CyclicReference private (val denot: SymDenotation)(implicit val ctx: Conte if (cycleSym.is(Implicit, butNot = Method) && cycleSym.owner.isTerm) CyclicReferenceInvolvingImplicit(cycleSym) else - errorMsg(this.ctx) + errorMsg(ctx) } } From e9ef0a7f99974a7c92d0737fe8f45bc977d13511 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 26 Apr 2018 21:12:28 +0200 Subject: [PATCH 09/21] Fix^2 context in CyclicReference toMessage --- .../dotty/tools/dotc/core/TypeErrors.scala | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala index edb8e374a597..9b6cf53eea7b 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala @@ -88,20 +88,21 @@ object handleRecursive { class CyclicReference private (val denot: SymDenotation) extends TypeError { - def errorMsg(cx: Context): Message = - if (cx.mode is Mode.InferringReturnType) { - cx.tree match { - case tree: untpd.DefDef if !tree.tpt.typeOpt.exists => - OverloadedOrRecursiveMethodNeedsResultType(tree.name) - case tree: untpd.ValDef if !tree.tpt.typeOpt.exists => - RecursiveValueNeedsResultType(tree.name) - case _ => - errorMsg(cx.outer) + override def toMessage(implicit ctx: Context) = { + + def errorMsg(cx: Context): Message = + if (cx.mode is Mode.InferringReturnType) { + cx.tree match { + case tree: untpd.DefDef if !tree.tpt.typeOpt.exists => + OverloadedOrRecursiveMethodNeedsResultType(tree.name) + case tree: untpd.ValDef if !tree.tpt.typeOpt.exists => + RecursiveValueNeedsResultType(tree.name) + case _ => + errorMsg(cx.outer) + } } - } - else CyclicReferenceInvolving(denot) + else CyclicReferenceInvolving(denot) - override def toMessage(implicit ctx: Context) = { val cycleSym = denot.symbol if (cycleSym.is(Implicit, butNot = Method) && cycleSym.owner.isTerm) CyclicReferenceInvolvingImplicit(cycleSym) From 0daaab048622375368e260ac3fdf4dcebe4a9ea1 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 26 Apr 2018 23:28:05 +0200 Subject: [PATCH 10/21] Fix error annotation in test --- tests/neg-custom-args/i4372.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/neg-custom-args/i4372.scala b/tests/neg-custom-args/i4372.scala index ebe4b1192d6b..e6e75061bb02 100644 --- a/tests/neg-custom-args/i4372.scala +++ b/tests/neg-custom-args/i4372.scala @@ -1,3 +1,3 @@ object i4372 { - class X[A >: X[_ <: X[_]] <: X[A]] + class X[A >: X[_ <: X[_]] <: X[A]] // error: too deep } From 03b9eb34c96d5c0bcf21107e3345f4b1bacfbd40 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 27 Apr 2018 10:20:38 +0200 Subject: [PATCH 11/21] MergeError refactorings --- .../dotty/tools/dotc/core/Denotations.scala | 44 +++++-------------- .../dotty/tools/dotc/core/TypeErrors.scala | 22 +++++++++- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index 0443f346476e..54993ce45cf6 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -332,21 +332,8 @@ object Denotations { } /** Handle merge conflict by throwing a `MergeError` exception */ - private def mergeConflict(tp1: Type, tp2: Type, that: Denotation)(implicit ctx: Context): Type = { - def showSymbol(sym: Symbol): String = if (sym.exists) sym.showLocated else "[unknown]" - def showType(tp: Type) = tp match { - case ClassInfo(_, cls, _, _, _) => cls.showLocated - case bounds: TypeBounds => i"type bounds $bounds" - case _ => tp.show - } - val msg = - s"""cannot merge - | ${showSymbol(this.symbol)} of type ${showType(tp1)} and - | ${showSymbol(that.symbol)} of type ${showType(tp2)} - """ - if (true) throw new MergeError(msg, tp1, tp2) - else throw new Error(msg) // flip condition for debugging - } + private def mergeConflict(tp1: Type, tp2: Type, that: Denotation)(implicit ctx: Context): Type = + throw new MergeError(this.symbol, that.symbol, tp1, tp2, NoPrefix) /** Merge parameter names of lambda types. If names in corresponding positions match, keep them, * otherwise generate new synthetic names. @@ -537,8 +524,7 @@ object Denotations { else if (pre.widen.classSymbol.is(Scala2x) || ctx.scala2Mode) info1 // follow Scala2 linearization - // compare with way merge is performed in SymDenotation#computeMembersNamed - else - throw new MergeError(s"${ex.getMessage} as members of type ${pre.show}", ex.tp1, ex.tp2) + else throw new MergeError(ex.sym1, ex.sym2, ex.tp1, ex.tp2, pre) } new JointRefDenotation(sym, jointInfo, denot1.validFor & denot2.validFor) } @@ -1136,21 +1122,15 @@ object Denotations { def doubleDefError(denot1: Denotation, denot2: Denotation, pre: Type = NoPrefix)(implicit ctx: Context): Nothing = { val sym1 = denot1.symbol val sym2 = denot2.symbol - def fromWhere = if (pre == NoPrefix) "" else i"\nwhen seen as members of $pre" - val msg = - if (denot1.isTerm) - i"""cannot merge - | $sym1: ${sym1.info} and - | $sym2: ${sym2.info}; - |they are both defined in ${sym1.owner} but have matching signatures - | ${denot1.info} and - | ${denot2.info}$fromWhere""" - else - i"""cannot merge - | $sym1 ${denot1.info} - | $sym2 ${denot2.info} - |they are conflicting definitions$fromWhere""" - throw new MergeError(msg, denot2.info, denot2.info) + if (denot1.isTerm) + throw new MergeError(sym1, sym2, sym1.info, sym2.info, pre) { + override def addendum(implicit ctx: Context) = + i""" + |they are both defined in ${sym1.owner} but have matching signatures + | ${denot1.info} and + | ${denot2.info}${super.addendum}""" + } + else throw new MergeError(sym1, sym2, denot1.info, denot2.info, pre) } // --- Overloaded denotations and predenotations ------------------------------------------------- diff --git a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala index 9b6cf53eea7b..e9fbe16cb1a8 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala @@ -123,4 +123,24 @@ object CyclicReference { } } -class MergeError(msg: String, val tp1: Type, val tp2: Type) extends TypeError(msg) \ No newline at end of file +class MergeError(val sym1: Symbol, val sym2: Symbol, val tp1: Type, val tp2: Type, prefix: Type) extends TypeError { + + private def showSymbol(sym: Symbol)(implicit ctx: Context): String = + if (sym.exists) sym.showLocated else "[unknown]" + + private def showType(tp: Type)(implicit ctx: Context) = tp match { + case ClassInfo(_, cls, _, _, _) => cls.showLocated + case _ => tp.show + } + + protected def addendum(implicit ctx: Context) = + if (prefix `eq` NoPrefix) "" else i"\nas members of type $prefix" + + override def toMessage(implicit ctx: Context): Message = { + if (ctx.debug) printStackTrace() + i"""cannot merge + | ${showSymbol(sym1)} of type ${showType(tp1)} and + | ${showSymbol(sym2)} of type ${showType(tp2)}$addendum + """ + } +} From e989cadf9cd86f0f16372d07329e33945f2d6574 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 27 Apr 2018 10:24:06 +0200 Subject: [PATCH 12/21] Add `append` method to Message Not needed here, but I missed the functionality elsewhere. --- .../src/dotty/tools/dotc/reporting/diagnostic/Message.scala | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/Message.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/Message.scala index 09d7ae9751be..74b4e1fe0194 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/Message.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/Message.scala @@ -65,6 +65,12 @@ abstract class Message(val errorId: ErrorMessageID) { self => val kind = self.kind val explanation = self.explanation } + + def append(suffix: => String): Message = new Message(errorId) { + val msg = self.msg ++ suffix + val kind = self.kind + val explanation = self.explanation + } } /** An extended message keeps the contained message from being evaluated, while From c05e35314bb69396837fdd2bc5469735d671d3d5 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 27 Apr 2018 10:59:53 +0200 Subject: [PATCH 13/21] Also catch TypeErrors in all transforms Once we are at it, let's try to bullet-proof everything. --- .../dotty/tools/dotc/core/TypeErrors.scala | 1 + .../tools/dotc/transform/MacroTransform.scala | 33 +++++++++++-------- .../tools/dotc/transform/MegaPhase.scala | 8 ++++- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala index e9fbe16cb1a8..3028ebc04a2d 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala @@ -66,6 +66,7 @@ class RecursionOverflow(val op: String, details: => String, previous: Throwable, val mostCommon = recursions.groupBy(_.op).toList.maxBy(_._2.map(_.weight).sum)._2.reverse s"""Recursion limit exceeded. |Maybe there is an illegal cyclic reference? + |If that's not the case, you could also try to increase the stacksize using the -Xss JVM option. |A recurring operation is (inner to outer): |${opsString(mostCommon)}""".stripMargin } diff --git a/compiler/src/dotty/tools/dotc/transform/MacroTransform.scala b/compiler/src/dotty/tools/dotc/transform/MacroTransform.scala index 9d84a2adb66c..5c3dea9cb79e 100644 --- a/compiler/src/dotty/tools/dotc/transform/MacroTransform.scala +++ b/compiler/src/dotty/tools/dotc/transform/MacroTransform.scala @@ -46,22 +46,27 @@ abstract class MacroTransform extends Phase { flatten(trees.mapconserve(transformStat(_))) } - override def transform(tree: Tree)(implicit ctx: Context): Tree = { - tree match { - case EmptyValDef => + override def transform(tree: Tree)(implicit ctx: Context): Tree = + try + tree match { + case EmptyValDef => + tree + case _: PackageDef | _: MemberDef => + super.transform(tree)(localCtx(tree)) + case impl @ Template(constr, parents, self, _) => + cpy.Template(tree)( + transformSub(constr), + transform(parents)(ctx.superCallContext), + transformSelf(self), + transformStats(impl.body, tree.symbol)) + case _ => + super.transform(tree) + } + catch { + case ex: TypeError => + ctx.error(ex.toMessage, tree.pos) tree - case _: PackageDef | _: MemberDef => - super.transform(tree)(localCtx(tree)) - case impl @ Template(constr, parents, self, _) => - cpy.Template(tree)( - transformSub(constr), - transform(parents)(ctx.superCallContext), - transformSelf(self), - transformStats(impl.body, tree.symbol)) - case _ => - super.transform(tree) } - } def transformSelf(vd: ValDef)(implicit ctx: Context) = cpy.ValDef(vd)(tpt = transform(vd.tpt)) diff --git a/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala b/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala index 868c48b0c0ae..dac467a20e86 100644 --- a/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala +++ b/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala @@ -193,7 +193,13 @@ class MegaPhase(val miniPhases: Array[MiniPhase]) extends Phase { case tree: Alternative => goAlternative(tree, start) case tree => goOther(tree, start) } - if (tree.isInstanceOf[NameTree]) goNamed(tree, start) else goUnnamed(tree, start) + try + if (tree.isInstanceOf[NameTree]) goNamed(tree, start) else goUnnamed(tree, start) + catch { + case ex: TypeError => + ctx.error(ex.toMessage, tree.pos) + tree + } } /** Transform full tree using all phases in this group that have idxInGroup >= start */ From a84b75f379a423acfc22a4d05b9728692b68fb44 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 27 Apr 2018 12:31:53 +0200 Subject: [PATCH 14/21] rearrange try-catch in MegaPhase Move it from `transformNode` to `goNamed` and `goUnnamed`. This might be faster since that way `transformNode` should be an inline candidate since it is short. --- .../tools/dotc/transform/MegaPhase.scala | 94 ++++++++++--------- 1 file changed, 51 insertions(+), 43 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala b/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala index dac467a20e86..8e1981c85e07 100644 --- a/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala +++ b/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala @@ -157,49 +157,57 @@ class MegaPhase(val miniPhases: Array[MiniPhase]) extends Phase { /** Transform node using all phases in this group that have idxInGroup >= start */ def transformNode(tree: Tree, start: Int)(implicit ctx: Context): Tree = { - def goNamed(tree: Tree, start: Int) = tree match { - case tree: Ident => goIdent(tree, start) - case tree: Select => goSelect(tree, start) - case tree: ValDef => goValDef(tree, start) - case tree: DefDef => goDefDef(tree, start) - case tree: TypeDef => goTypeDef(tree, start) - case tree: Bind => goBind(tree, start) - case _ => goOther(tree, start) - } - def goUnnamed(tree: Tree, start: Int) = tree match { - case tree: Apply => goApply(tree, start) - case tree: TypeTree => goTypeTree(tree, start) - case tree: Thicket => - cpy.Thicket(tree)(tree.trees.mapConserve(transformNode(_, start))) - case tree: This => goThis(tree, start) - case tree: Literal => goLiteral(tree, start) - case tree: Block => goBlock(tree, start) - case tree: TypeApply => goTypeApply(tree, start) - case tree: If => goIf(tree, start) - case tree: New => goNew(tree, start) - case tree: Typed => goTyped(tree, start) - case tree: CaseDef => goCaseDef(tree, start) - case tree: Closure => goClosure(tree, start) - case tree: Assign => goAssign(tree, start) - case tree: SeqLiteral => goSeqLiteral(tree, start) - case tree: Super => goSuper(tree, start) - case tree: Template => goTemplate(tree, start) - case tree: Match => goMatch(tree, start) - case tree: UnApply => goUnApply(tree, start) - case tree: PackageDef => goPackageDef(tree, start) - case tree: Try => goTry(tree, start) - case tree: Inlined => goInlined(tree, start) - case tree: Return => goReturn(tree, start) - case tree: Alternative => goAlternative(tree, start) - case tree => goOther(tree, start) - } - try - if (tree.isInstanceOf[NameTree]) goNamed(tree, start) else goUnnamed(tree, start) - catch { - case ex: TypeError => - ctx.error(ex.toMessage, tree.pos) - tree - } + def goNamed(tree: Tree, start: Int) = + try + tree match { + case tree: Ident => goIdent(tree, start) + case tree: Select => goSelect(tree, start) + case tree: ValDef => goValDef(tree, start) + case tree: DefDef => goDefDef(tree, start) + case tree: TypeDef => goTypeDef(tree, start) + case tree: Bind => goBind(tree, start) + case _ => goOther(tree, start) + } + catch { + case ex: TypeError => + ctx.error(ex.toMessage, tree.pos) + tree + } + def goUnnamed(tree: Tree, start: Int) = + try + tree match { + case tree: Apply => goApply(tree, start) + case tree: TypeTree => goTypeTree(tree, start) + case tree: Thicket => + cpy.Thicket(tree)(tree.trees.mapConserve(transformNode(_, start))) + case tree: This => goThis(tree, start) + case tree: Literal => goLiteral(tree, start) + case tree: Block => goBlock(tree, start) + case tree: TypeApply => goTypeApply(tree, start) + case tree: If => goIf(tree, start) + case tree: New => goNew(tree, start) + case tree: Typed => goTyped(tree, start) + case tree: CaseDef => goCaseDef(tree, start) + case tree: Closure => goClosure(tree, start) + case tree: Assign => goAssign(tree, start) + case tree: SeqLiteral => goSeqLiteral(tree, start) + case tree: Super => goSuper(tree, start) + case tree: Template => goTemplate(tree, start) + case tree: Match => goMatch(tree, start) + case tree: UnApply => goUnApply(tree, start) + case tree: PackageDef => goPackageDef(tree, start) + case tree: Try => goTry(tree, start) + case tree: Inlined => goInlined(tree, start) + case tree: Return => goReturn(tree, start) + case tree: Alternative => goAlternative(tree, start) + case tree => goOther(tree, start) + } + catch { + case ex: TypeError => + ctx.error(ex.toMessage, tree.pos) + tree + } + if (tree.isInstanceOf[NameTree]) goNamed(tree, start) else goUnnamed(tree, start) } /** Transform full tree using all phases in this group that have idxInGroup >= start */ From 5ade7fecee6aadffa72616ef20c93f38594ef041 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Wed, 9 May 2018 19:00:06 +0200 Subject: [PATCH 15/21] Preload handleRecursive to prevent nested stackoverflows Thanks `@smarter` for guidance. --- compiler/src/dotty/tools/dotc/core/TypeErrors.scala | 5 +++++ compiler/src/dotty/tools/package.scala | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala index 3028ebc04a2d..cbfd915dea02 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala @@ -75,6 +75,11 @@ class RecursionOverflow(val op: String, details: => String, previous: Throwable, override def getStackTrace() = previous.getStackTrace() } +/** Post-process exceptions that might result from StackOverflow to add + * tracing information while unwalking the stack. + */ +// Beware: Since this object is only used when handling a StackOverflow, this code +// cannot consume significant amounts of stack. object handleRecursive { def apply(op: String, details: => String, exc: Throwable, weight: Int = 1): Nothing = exc match { case _: RecursionOverflow => diff --git a/compiler/src/dotty/tools/package.scala b/compiler/src/dotty/tools/package.scala index 68d24e2297e0..220eb6b7588b 100644 --- a/compiler/src/dotty/tools/package.scala +++ b/compiler/src/dotty/tools/package.scala @@ -5,6 +5,11 @@ package object tools { class sharable extends Annotation class unshared extends Annotation + // Ensure this object is already classloaded, since it's only actually used + // when handling stack overflows and every operation (including class loading) + // risks failing. + dotty.tools.dotc.core.handleRecursive + val ListOfNil = Nil :: Nil /** True if two lists have the same length. Since calling length on linear sequences From ecbd6926755f2efbbe8b2cdcc97259059166bee2 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Wed, 9 May 2018 21:00:53 +0200 Subject: [PATCH 16/21] Ensure `-YshowSuppressedErrors` also affects `UniqueMessagePositions` As a followup to the earlier bugfix. --- .../src/dotty/tools/dotc/reporting/UniqueMessagePositions.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/UniqueMessagePositions.scala b/compiler/src/dotty/tools/dotc/reporting/UniqueMessagePositions.scala index 6fd971c2a4c0..b7049f8038b6 100644 --- a/compiler/src/dotty/tools/dotc/reporting/UniqueMessagePositions.scala +++ b/compiler/src/dotty/tools/dotc/reporting/UniqueMessagePositions.scala @@ -18,7 +18,7 @@ trait UniqueMessagePositions extends Reporter { */ override def isHidden(m: MessageContainer)(implicit ctx: Context): Boolean = super.isHidden(m) || { - m.pos.exists && { + m.pos.exists && !ctx.settings.YshowSuppressedErrors.value && { var shouldHide = false for (pos <- m.pos.start to m.pos.end) { positions get (ctx.source, pos) match { From 5ef9263aff757ce46f5ec891cb496503e2c4a0de Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Tue, 29 May 2018 16:50:31 +0200 Subject: [PATCH 17/21] Fix typo in modified (commented-out) code --- compiler/src/dotty/tools/dotc/typer/RefChecks.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index af7dbdaed390..df01fe989637 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -1614,7 +1614,7 @@ class RefChecks extends MiniPhase { thisPhase => } catch { case ex: TypeError => if (settings.debug) ex.printStackTrace() - unit.error(tree.pos, ex.toMssage) + unit.error(tree.pos, ex.toMessage) tree } finally { localTyper = savedLocalTyper From 34c4107df4a79dfc74b31ba085caebc46f6344b4 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Tue, 29 May 2018 17:00:39 +0200 Subject: [PATCH 18/21] Add a flag to disable handleRecursive --- .../tools/dotc/config/ScalaSettings.scala | 1 + .../dotty/tools/dotc/core/TypeErrors.scala | 22 ++++++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 1328a6d5d812..da9fa5f84ed5 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -146,6 +146,7 @@ class ScalaSettings extends Settings.SettingGroup { val Xlink = BooleanSetting("-Xlink", "Recompile library code with the application.") val YoptPhases = PhasesSetting("-Yopt-phases", "Restrict the optimisation phases to execute under -optimise.") val YoptFuel = IntSetting("-Yopt-fuel", "Maximum number of optimisations performed under -optimise.", -1) + val YnoDecodeStacktraces = BooleanSetting("-Yno-decode-stacktraces", "Show operations that triggered StackOverflows instead of printing the raw stacktraces.") /** Dottydoc specific settings */ val siteRoot = StringSetting( diff --git a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala index cbfd915dea02..9218dd8a4267 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala @@ -81,14 +81,20 @@ class RecursionOverflow(val op: String, details: => String, previous: Throwable, // Beware: Since this object is only used when handling a StackOverflow, this code // cannot consume significant amounts of stack. object handleRecursive { - def apply(op: String, details: => String, exc: Throwable, weight: Int = 1): Nothing = exc match { - case _: RecursionOverflow => - throw new RecursionOverflow(op, details, exc, weight) - case _ => - var e = exc - while (e != null && !e.isInstanceOf[StackOverflowError]) e = e.getCause - if (e != null) throw new RecursionOverflow(op, details, e, weight) - else throw exc + def apply(op: String, details: => String, exc: Throwable, weight: Int = 1)(implicit ctx: Context): Nothing = { + if (ctx.settings.YnoDecodeStacktraces.value) { + throw exc + } else { + exc match { + case _: RecursionOverflow => + throw new RecursionOverflow(op, details, exc, weight) + case _ => + var e = exc + while (e != null && !e.isInstanceOf[StackOverflowError]) e = e.getCause + if (e != null) throw new RecursionOverflow(op, details, e, weight) + else throw exc + } + } } } From 7814a446bc2743fb8dbf5ec549417142509d4d12 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Tue, 29 May 2018 16:37:47 +0200 Subject: [PATCH 19/21] Typer: Refine position when catching top-level TypeError Ensures stackoverflows aren't hidden by errors, without needing #4511. Also add relevant testcase from https://github.com/lampepfl/dotty/pull/4385#issuecomment-387847237; I confirmed this fails without the position fix. --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 8 +++++++- tests/neg/i4385.scala | 7 +++++++ 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 tests/neg/i4385.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 289542c54817..0cbf417e0d19 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1862,7 +1862,13 @@ class Typer extends Namer assertPositioned(tree) try adapt(typedUnadapted(tree, pt, locked), pt, locked) catch { - case ex: TypeError => errorTree(tree, ex.toMessage) + case ex: TypeError => + // This is equivalent to errorTree(tree, ex.toMessage) but + // uses tree.pos.focus instead of tree.pos. We use this because: + // - since tree can be a top-level definition, tree.pos can point to the whole definition + // - that would in turn hide all other type errors inside tree. + // TODO: might be even better to store positions inside TypeErrors. + tree withType errorType(ex.toMessage, tree.pos.focus) } } diff --git a/tests/neg/i4385.scala b/tests/neg/i4385.scala new file mode 100644 index 000000000000..be40f9e553f3 --- /dev/null +++ b/tests/neg/i4385.scala @@ -0,0 +1,7 @@ +class i0 { // error: stack overflow. + class i1 { type i2 } + type i3 = i1.i2 // error + type i4 = i0 { type i1 <: i4 } // this line triggers the overflow + // (and reporting it here would be better). + // This test ensure the above stack overflow is reported and not hidden by the earlier error. +} From 640e0a855eae26c92185215307d964f8576e120b Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Wed, 30 May 2018 02:47:18 +0200 Subject: [PATCH 20/21] Address review feedback --- compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala | 5 ++++- compiler/src/dotty/tools/dotc/typer/Typer.scala | 5 ++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala index 79799c9013a1..923425800e93 100644 --- a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala +++ b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala @@ -15,8 +15,11 @@ object ErrorReporting { import tpd._ + def errorTree(tree: untpd.Tree, msg: => Message, pos: Position)(implicit ctx: Context): tpd.Tree = + tree withType errorType(msg, pos) + def errorTree(tree: untpd.Tree, msg: => Message)(implicit ctx: Context): tpd.Tree = - tree withType errorType(msg, tree.pos) + errorTree(tree, msg, tree.pos) def errorType(msg: => Message, pos: Position)(implicit ctx: Context): ErrorType = { ctx.error(msg, pos) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 0cbf417e0d19..6a5b48b08522 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1863,12 +1863,11 @@ class Typer extends Namer try adapt(typedUnadapted(tree, pt, locked), pt, locked) catch { case ex: TypeError => - // This is equivalent to errorTree(tree, ex.toMessage) but - // uses tree.pos.focus instead of tree.pos. We use this because: + errorTree(tree, ex.toMessage, tree.pos.focus) + // This uses tree.pos.focus instead of the default tree.pos, because: // - since tree can be a top-level definition, tree.pos can point to the whole definition // - that would in turn hide all other type errors inside tree. // TODO: might be even better to store positions inside TypeErrors. - tree withType errorType(ex.toMessage, tree.pos.focus) } } From 7fea20774f89ad11c728546e11ffa54e06128def Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Mon, 4 Jun 2018 18:47:05 +0200 Subject: [PATCH 21/21] Negate description of negated option --- compiler/src/dotty/tools/dotc/config/ScalaSettings.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index da9fa5f84ed5..376f89e4f96c 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -146,7 +146,7 @@ class ScalaSettings extends Settings.SettingGroup { val Xlink = BooleanSetting("-Xlink", "Recompile library code with the application.") val YoptPhases = PhasesSetting("-Yopt-phases", "Restrict the optimisation phases to execute under -optimise.") val YoptFuel = IntSetting("-Yopt-fuel", "Maximum number of optimisations performed under -optimise.", -1) - val YnoDecodeStacktraces = BooleanSetting("-Yno-decode-stacktraces", "Show operations that triggered StackOverflows instead of printing the raw stacktraces.") + val YnoDecodeStacktraces = BooleanSetting("-Yno-decode-stacktraces", "Show raw StackOverflow stacktraces, instead of decoding them into triggering operations.") /** Dottydoc specific settings */ val siteRoot = StringSetting(