From ec07deeaca4ee6138f7f9cea64251fd4f7988531 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 12 Dec 2017 18:09:21 +0100 Subject: [PATCH 1/3] Fix #3538: Better invalidation of typed arguments in FunProtos One scenario we overlooked was that an argument in a FunProto might have been computed in a context that was subsequently retracted from its typerstate (e.g. because an enclosing subtype test failed). In that case we need to re-evaluate the argument in order to avoid orphan typerefs. --- .../src/dotty/tools/dotc/config/Config.scala | 2 +- .../dotty/tools/dotc/core/Constraint.scala | 6 +++++ .../tools/dotc/core/OrderingConstraint.scala | 8 +++++++ .../dotty/tools/dotc/core/TypeComparer.scala | 4 ++-- .../dotty/tools/dotc/core/TyperState.scala | 11 ++++++++-- .../dotty/tools/dotc/typer/ProtoTypes.scala | 22 ++++++++++--------- tests/pos/i3538.scala | 15 +++++++++++++ 7 files changed, 53 insertions(+), 15 deletions(-) create mode 100644 tests/pos/i3538.scala diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala index 60108d51d236..4e4156079026 100644 --- a/compiler/src/dotty/tools/dotc/config/Config.scala +++ b/compiler/src/dotty/tools/dotc/config/Config.scala @@ -148,7 +148,7 @@ object Config { final val showCompletions = false /** If set, enables tracing */ - final val tracingEnabled = false + final val tracingEnabled = true /** Initial capacity of uniques HashMap. * Note: This MUST BE a power of two to work with util.HashSet diff --git a/compiler/src/dotty/tools/dotc/core/Constraint.scala b/compiler/src/dotty/tools/dotc/core/Constraint.scala index ffa5082bf5b2..605134f40c3f 100644 --- a/compiler/src/dotty/tools/dotc/core/Constraint.scala +++ b/compiler/src/dotty/tools/dotc/core/Constraint.scala @@ -144,4 +144,10 @@ abstract class Constraint extends Showable { /** Check that constraint only refers to TypeParamRefs bound by itself */ def checkClosed()(implicit ctx: Context): Unit + + /** Constraint has not yet been retracted from a typer state */ + def isRetracted(): Boolean + + /** Indicate that constraint has been retracted from a typer state */ + def markRetracted(): Unit } diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index 5062bf5c9431..2bdb8aa130ce 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -570,6 +570,14 @@ class OrderingConstraint(private val boundsMap: ParamBounds, private def checkNonCyclic(param: TypeParamRef)(implicit ctx: Context): Unit = assert(!isLess(param, param), i"cyclic constraint involving $param in $this") +// ---------- Invalidation ------------------------------------------- + + private var retracted = false + + def isRetracted: Boolean = retracted + + def markRetracted(): Unit = retracted = true + // ---------- toText ----------------------------------------------------- override def toText(printer: Printer): Text = { diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index ce4f6dd72237..cf36629dd599 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -118,7 +118,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling { if (recCount < Config.LogPendingSubTypesThreshold) firstTry(tp1, tp2) else monitoredIsSubType(tp1, tp2) recCount = recCount - 1 - if (!result) constraint = saved + if (!result) state.resetConstraintTo(saved) else if (recCount == 0 && needsGc) { state.gc() needsGc = false @@ -129,7 +129,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling { case NonFatal(ex) => if (ex.isInstanceOf[AssertionError]) showGoal(tp1, tp2) recCount -= 1 - constraint = saved + state.resetConstraintTo(saved) successCount = savedSuccessCount throw ex } diff --git a/compiler/src/dotty/tools/dotc/core/TyperState.scala b/compiler/src/dotty/tools/dotc/core/TyperState.scala index 532d7f90d408..77dbee3d8cc2 100644 --- a/compiler/src/dotty/tools/dotc/core/TyperState.scala +++ b/compiler/src/dotty/tools/dotc/core/TyperState.scala @@ -12,6 +12,7 @@ import printing.Texts._ import config.Config import collection.mutable import java.lang.ref.WeakReference +import Decorators._ class TyperState(previous: TyperState /* | Null */) extends DotClass with Showable { @@ -29,10 +30,16 @@ class TyperState(previous: TyperState /* | Null */) extends DotClass with Showab def constraint = myConstraint def constraint_=(c: Constraint)(implicit ctx: Context) = { +// println(i"assigning $c to $hashesStr") if (Config.debugCheckConstraintsClosed && isGlobalCommittable) c.checkClosed() myConstraint = c } + def resetConstraintTo(c: Constraint) = { + if (c `ne` myConstraint) myConstraint.markRetracted() + myConstraint = c + } + private val previousConstraint = if (previous == null) constraint else previous.constraint @@ -90,8 +97,8 @@ class TyperState(previous: TyperState /* | Null */) extends DotClass with Showab /** Test using `op`, restoring typerState to previous state afterwards */ def test[T](op: => T): T = { - val savedReporter = myReporter val savedConstraint = myConstraint + val savedReporter = myReporter val savedCommittable = myIsCommittable val savedCommitted = isCommitted myIsCommittable = false @@ -105,8 +112,8 @@ class TyperState(previous: TyperState /* | Null */) extends DotClass with Showab } try op finally { + resetConstraintTo(savedConstraint) myReporter = savedReporter - myConstraint = savedConstraint myIsCommittable = savedCommittable isCommitted = savedCommitted } diff --git a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala index 678e1ca99222..122434fb3dda 100644 --- a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala +++ b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala @@ -67,7 +67,7 @@ object ProtoTypes { case _ => true } - if (!res) ctx.typerState.constraint = savedConstraint + if (!res) ctx.typerState.resetConstraintTo(savedConstraint) res } } @@ -183,8 +183,8 @@ object ProtoTypes { /** A map in which typed arguments can be stored to be later integrated in `typedArgs`. */ private[this] var myTypedArg: SimpleIdentityMap[untpd.Tree, Tree] = SimpleIdentityMap.Empty - /** A map recording the typer states in which arguments stored in myTypedArg were typed */ - private[this] var evalState: SimpleIdentityMap[untpd.Tree, TyperState] = SimpleIdentityMap.Empty + /** A map recording the typer states and constraints in which arguments stored in myTypedArg were typed */ + private[this] var evalState: SimpleIdentityMap[untpd.Tree, (TyperState, Constraint)] = SimpleIdentityMap.Empty def isMatchedBy(tp: Type)(implicit ctx: Context) = typer.isApplicable(tp, Nil, typedArgs, resultType) @@ -195,17 +195,19 @@ object ProtoTypes { override def notApplied = WildcardType - /** Forget the types of any arguments that have been typed producing a constraint in a - * typer state that is not yet committed into the one of the current context `ctx`. + /** Forget the types of any arguments that have been typed producing a constraint that is + * either + * - in a typer state that is not yet committed into the one of the current context `ctx`, or + * - has been retracted from its typestate because oif a failed operation. * This is necessary to avoid "orphan" TypeParamRefs that are referred to from * type variables in the typed arguments, but that are not registered in the - * current constraint. A test case is pos/t1756.scala. + * current constraint. Test cases are pos/t1756.scala and pos/i3538.scala. * @return True if all arguments have types (in particular, no types were forgotten). */ def allArgTypesAreCurrent()(implicit ctx: Context): Boolean = { - evalState foreachBinding { (arg, tstate) => - if (tstate.uncommittedAncestor.constraint ne ctx.typerState.constraint) { - typr.println(i"need to invalidate $arg / ${myTypedArg(arg)}, ${tstate.constraint}, current = ${ctx.typerState.constraint}") + evalState foreachBinding { (arg, tstateConstr) => + if ((tstateConstr._1.uncommittedAncestor.constraint `ne` ctx.typerState.constraint) || + tstateConstr._2.isRetracted) { myTypedArg = myTypedArg.remove(arg) evalState = evalState.remove(arg) } @@ -219,7 +221,7 @@ object ProtoTypes { targ = typerFn(arg) if (!ctx.reporter.hasPending) { myTypedArg = myTypedArg.updated(arg, targ) - evalState = evalState.updated(arg, ctx.typerState) + evalState = evalState.updated(arg, (ctx.typerState, ctx.typerState.constraint)) } } targ diff --git a/tests/pos/i3538.scala b/tests/pos/i3538.scala new file mode 100644 index 000000000000..f6816f3e52b1 --- /dev/null +++ b/tests/pos/i3538.scala @@ -0,0 +1,15 @@ + +class Inv[T] + +class Test { + implicit class Wrong(test: Test) { + def right: Any = ??? + } + implicit class Right(test: Test) { + def right(node: Any): Any = ??? + } + + def inv[T](x: T): Inv[T] = ??? + + (this).right(inv(1)) +} From 7c608c4410d3045ccf6da653ae3761e5bfaf39ea Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 13 Dec 2017 00:10:30 +0100 Subject: [PATCH 2/3] Fix () typo --- compiler/src/dotty/tools/dotc/core/Constraint.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/Constraint.scala b/compiler/src/dotty/tools/dotc/core/Constraint.scala index 605134f40c3f..57aaa8d68ee2 100644 --- a/compiler/src/dotty/tools/dotc/core/Constraint.scala +++ b/compiler/src/dotty/tools/dotc/core/Constraint.scala @@ -146,7 +146,7 @@ abstract class Constraint extends Showable { def checkClosed()(implicit ctx: Context): Unit /** Constraint has not yet been retracted from a typer state */ - def isRetracted(): Boolean + def isRetracted: Boolean /** Indicate that constraint has been retracted from a typer state */ def markRetracted(): Unit From d8b7566fafe087faee2cfcabd5fd6810262b1a3d Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 13 Dec 2017 08:40:41 +0100 Subject: [PATCH 3/3] Polishings --- compiler/src/dotty/tools/dotc/config/Config.scala | 2 +- compiler/src/dotty/tools/dotc/core/TyperState.scala | 3 +-- compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala | 8 ++++---- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala index 4e4156079026..60108d51d236 100644 --- a/compiler/src/dotty/tools/dotc/config/Config.scala +++ b/compiler/src/dotty/tools/dotc/config/Config.scala @@ -148,7 +148,7 @@ object Config { final val showCompletions = false /** If set, enables tracing */ - final val tracingEnabled = true + final val tracingEnabled = false /** Initial capacity of uniques HashMap. * Note: This MUST BE a power of two to work with util.HashSet diff --git a/compiler/src/dotty/tools/dotc/core/TyperState.scala b/compiler/src/dotty/tools/dotc/core/TyperState.scala index 77dbee3d8cc2..c4fdd272b1a4 100644 --- a/compiler/src/dotty/tools/dotc/core/TyperState.scala +++ b/compiler/src/dotty/tools/dotc/core/TyperState.scala @@ -12,7 +12,6 @@ import printing.Texts._ import config.Config import collection.mutable import java.lang.ref.WeakReference -import Decorators._ class TyperState(previous: TyperState /* | Null */) extends DotClass with Showable { @@ -30,11 +29,11 @@ class TyperState(previous: TyperState /* | Null */) extends DotClass with Showab def constraint = myConstraint def constraint_=(c: Constraint)(implicit ctx: Context) = { -// println(i"assigning $c to $hashesStr") if (Config.debugCheckConstraintsClosed && isGlobalCommittable) c.checkClosed() myConstraint = c } + /** Reset constraint to `c` and mark current constraint as retracted if it differs from `c` */ def resetConstraintTo(c: Constraint) = { if (c `ne` myConstraint) myConstraint.markRetracted() myConstraint = c diff --git a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala index 122434fb3dda..067b9c1e15df 100644 --- a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala +++ b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala @@ -195,10 +195,9 @@ object ProtoTypes { override def notApplied = WildcardType - /** Forget the types of any arguments that have been typed producing a constraint that is - * either - * - in a typer state that is not yet committed into the one of the current context `ctx`, or - * - has been retracted from its typestate because oif a failed operation. + /** Forget the types of any arguments that have been typed producing a constraint + * - that is in a typer state that is not yet committed into the one of the current context `ctx`, + * - or that has been retracted from its typestate because oif a failed operation. * This is necessary to avoid "orphan" TypeParamRefs that are referred to from * type variables in the typed arguments, but that are not registered in the * current constraint. Test cases are pos/t1756.scala and pos/i3538.scala. @@ -208,6 +207,7 @@ object ProtoTypes { evalState foreachBinding { (arg, tstateConstr) => if ((tstateConstr._1.uncommittedAncestor.constraint `ne` ctx.typerState.constraint) || tstateConstr._2.isRetracted) { + typr.println(i"need to invalidate $arg / ${myTypedArg(arg)}, ${tstateConstr._2}, current = ${ctx.typerState.constraint}") myTypedArg = myTypedArg.remove(arg) evalState = evalState.remove(arg) }