From eb21d24043cc285238b7bb41518c777dc16af28c Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 3 Oct 2017 09:51:01 +0200 Subject: [PATCH 1/5] Fix GADT-related memory leak There was a confusion which led to the wrong gadt map being used for pattern-bound variables if there was no other GADT variable in the enclosing method. This led to the outermost gadt map in the initial context being populated with type bounds which never went away. --- .../src/dotty/tools/dotc/core/Contexts.scala | 9 +++++--- .../tools/dotc/transform/PostTyper.scala | 12 +++------- .../src/dotty/tools/dotc/typer/Typer.scala | 22 ++++++++----------- 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index e81628c21639..25339e05a9a5 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -476,6 +476,7 @@ object Contexts { def setRunInfo(runInfo: RunInfo): this.type = { this.runInfo = runInfo; this } def setDiagnostics(diagnostics: Option[StringBuilder]): this.type = { this.diagnostics = diagnostics; this } def setGadt(gadt: GADTMap): this.type = { this.gadt = gadt; this } + def setFreshGADTBounds: this.type = setGadt(new GADTMap(gadt.bounds)) def setTypeComparerFn(tcfn: Context => TypeComparer): this.type = { this.typeComparer = tcfn(this); this } def setSearchHistory(searchHistory: SearchHistory): this.type = { this.searchHistory = searchHistory; this } def setFreshNames(freshNames: FreshNameCreator): this.type = { this.freshNames = freshNames; this } @@ -493,7 +494,6 @@ object Contexts { def setSetting[T](setting: Setting[T], value: T): this.type = setSettings(setting.updateIn(sstate, value)) - def setFreshGADTBounds: this.type = { this.gadt = new GADTMap(gadt.bounds); this } def setDebug = setSetting(base.settings.debug, true) } @@ -532,7 +532,7 @@ object Contexts { moreProperties = Map.empty typeComparer = new TypeComparer(this) searchHistory = new SearchHistory(0, Map()) - gadt = new GADTMap(SimpleMap.Empty) + gadt = new GADTMap(SimpleMap.Empty) // EmptyGADTMap } @sharable object NoContext extends Context { @@ -694,10 +694,13 @@ object Contexts { implicit val ctx: Context = initctx } - class GADTMap(initBounds: SimpleMap[Symbol, TypeBounds]) { + class GADTMap(initBounds: SimpleMap[Symbol, TypeBounds]) extends util.DotClass { private var myBounds = initBounds def setBounds(sym: Symbol, b: TypeBounds): Unit = myBounds = myBounds.updated(sym, b) def bounds = myBounds } + object EmptyGADTMap extends GADTMap(SimpleMap.Empty) { + override def setBounds(sym: Symbol, b: TypeBounds) = unsupported("EmptyGADTMap.setBound") + } } diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala index 530c11a3b7e7..2d7610e1fd1a 100644 --- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala @@ -54,17 +54,9 @@ import reporting.diagnostic.messages.SuperCallsNotAllowedInline * mini-phase or subfunction of a macro phase equally well. But taken by themselves * they do not warrant their own group of miniphases before pickling. */ -class PostTyper extends MacroTransform with SymTransformer { thisTransformer => - - +class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTransformer => import tpd._ - def transformSym(ref: SymDenotation)(implicit ctx: Context): SymDenotation = { - if (ref.is(BindDefinedType) && ctx.gadt.bounds.contains(ref.symbol)) { - ref.copySymDenotation(info = ctx.gadt.bounds.apply(ref.symbol) & ref.info) - } else ref - } - /** the following two members override abstract members in Transform */ override def phaseName: String = "posttyper" @@ -289,6 +281,8 @@ class PostTyper extends MacroTransform with SymTransformer { thisTransformer => case _ => } super.transform(tree) + case Typed(Ident(nme.WILDCARD), _) => + tree // skip checking pattern type case tree => super.transform(tree) } diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 9d7fe3e4486e..e93aea790f7a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -921,16 +921,21 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit def typedCase(tree: untpd.CaseDef, pt: Type, selType: Type, gadtSyms: Set[Symbol])(implicit ctx: Context): CaseDef = track("typedCase") { val originalCtx = ctx + val gadtCtx = ctx.fresh.setFreshGADTBounds + for (sym <- gadtSyms) + if (!gadtCtx.gadt.bounds.contains(sym)) + gadtCtx.gadt.setBounds(sym, TypeBounds.empty) + /** - replace all references to symbols associated with wildcards by their GADT bounds * - enter all symbols introduced by a Bind in current scope */ val indexPattern = new TreeMap { val elimWildcardSym = new TypeMap { def apply(t: Type) = t match { - case ref: TypeRef if ref.name == tpnme.WILDCARD && ctx.gadt.bounds.contains(ref.symbol) => - ctx.gadt.bounds(ref.symbol) - case TypeAlias(ref: TypeRef) if ref.name == tpnme.WILDCARD && ctx.gadt.bounds.contains(ref.symbol) => - ctx.gadt.bounds(ref.symbol) + case ref: TypeRef if ref.name == tpnme.WILDCARD && gadtCtx.gadt.bounds.contains(ref.symbol) => + gadtCtx.gadt.bounds(ref.symbol) + case TypeAlias(ref: TypeRef) if ref.name == tpnme.WILDCARD && gadtCtx.gadt.bounds.contains(ref.symbol) => + gadtCtx.gadt.bounds(ref.symbol) case _ => mapOver(t) } @@ -955,15 +960,6 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit assignType(cpy.CaseDef(tree)(pat1, guard1, body1), body1) } - val gadtCtx = - if (gadtSyms.isEmpty) ctx - else { - val c = ctx.fresh.setFreshGADTBounds - for (sym <- gadtSyms) - if (!c.gadt.bounds.contains(sym)) - c.gadt.setBounds(sym, TypeBounds.empty) - c - } val pat1 = typedPattern(tree.pat, selType)(gadtCtx) caseRest(pat1)(gadtCtx.fresh.setNewScope) } From dbe64c1f059a20bce4d846808ff37655394bd9f5 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 3 Oct 2017 12:04:17 +0200 Subject: [PATCH 2/5] Add @sharable annotation --- compiler/src/dotty/tools/dotc/core/Contexts.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index 25339e05a9a5..dbed4c17d572 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -700,7 +700,8 @@ object Contexts { myBounds = myBounds.updated(sym, b) def bounds = myBounds } - object EmptyGADTMap extends GADTMap(SimpleMap.Empty) { - override def setBounds(sym: Symbol, b: TypeBounds) = unsupported("EmptyGADTMap.setBound") + + @sharable object EmptyGADTMap extends GADTMap(SimpleMap.Empty) { + override def setBounds(sym: Symbol, b: TypeBounds) = unsupported("EmptyGADTMap.setBounds") } } From 3be3bea17f252705ff7d9f8a70bf55204c323af0 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 3 Oct 2017 17:18:44 +0200 Subject: [PATCH 3/5] Allow bench task to pause between runs If -Xprompt is set, pause a dotty.tools.dotc.Bench task between compiler runs. This is useful for, e.g. taking a memory snapshot at a predictable time. --- compiler/src/dotty/tools/dotc/Bench.scala | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/Bench.scala b/compiler/src/dotty/tools/dotc/Bench.scala index ee7789c32f9f..a5fe890b8119 100644 --- a/compiler/src/dotty/tools/dotc/Bench.scala +++ b/compiler/src/dotty/tools/dotc/Bench.scala @@ -1,7 +1,3 @@ -/* NSC -- new Scala compiler - * Copyright 2005-2013 LAMP/EPFL - * @author Martin Odersky - */ package dotty.tools package dotc @@ -24,6 +20,11 @@ object Bench extends Driver { val start = System.nanoTime() val r = super.doCompile(compiler, fileNames) println(s"time elapsed: ${(System.nanoTime - start) / 1000000}ms") + if (ctx.settings.prompt.value) { + print("hit to continue >") + System.in.read() + println() + } r } From 318cec329d6a0b05b945e54a847e2e0f1dbf5d04 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 3 Oct 2017 17:19:10 +0200 Subject: [PATCH 4/5] Clear useCount map after runs. --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index a937aa418d3d..9cececffd041 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -491,7 +491,10 @@ trait ImplicitRunInfo { self: RunInfo => override def default(key: TermRef) = 0 } - def clear() = implicitScopeCache.clear() + def clear() = { + implicitScopeCache.clear() + useCount.clear() + } } /** The implicit resolution part of type checking */ From cf15ca28e6d6a2af24a5930e5a87d1587b57895e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 4 Oct 2017 13:40:19 +0200 Subject: [PATCH 5/5] Address reviewer comments --- compiler/src/dotty/tools/dotc/core/Contexts.scala | 2 +- .../src/dotty/tools/dotc/transform/PostTyper.scala | 11 +++++++++-- compiler/src/dotty/tools/dotc/typer/Checking.scala | 4 ++-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index dbed4c17d572..247c028ae221 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -532,7 +532,7 @@ object Contexts { moreProperties = Map.empty typeComparer = new TypeComparer(this) searchHistory = new SearchHistory(0, Map()) - gadt = new GADTMap(SimpleMap.Empty) // EmptyGADTMap + gadt = EmptyGADTMap } @sharable object NoContext extends Context { diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala index 2d7610e1fd1a..0625242adbc0 100644 --- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala @@ -256,7 +256,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTrans case tree @ Annotated(annotated, annot) => cpy.Annotated(tree)(transform(annotated), transformAnnot(annot)) case tree: AppliedTypeTree => - Checking.checkAppliedType(tree) + Checking.checkAppliedType(tree, boundsCheck = !ctx.mode.is(Mode.Pattern)) super.transform(tree) case SingletonTypeTree(ref) => Checking.checkRealizable(ref.tpe, ref.pos.focus) @@ -282,7 +282,14 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTrans } super.transform(tree) case Typed(Ident(nme.WILDCARD), _) => - tree // skip checking pattern type + super.transform(tree)(ctx.addMode(Mode.Pattern)) + // The added mode signals that bounds in a pattern need not + // conform to selector bounds. I.e. assume + // type Tree[T >: Null <: Type] + // One is still allowed to write + // case x: Tree[_] + // (which translates to) + // case x: (_: Tree[_]) case tree => super.transform(tree) } diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 3c60db0e0ec1..27b685a19299 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -68,7 +68,7 @@ object Checking { * Unreducible applications correspond to general existentials, and we * cannot handle those. */ - def checkAppliedType(tree: AppliedTypeTree)(implicit ctx: Context) = { + def checkAppliedType(tree: AppliedTypeTree, boundsCheck: Boolean)(implicit ctx: Context) = { val AppliedTypeTree(tycon, args) = tree // If `args` is a list of named arguments, return corresponding type parameters, // otherwise return type parameters unchanged @@ -81,7 +81,7 @@ object Checking { val bounds = tparams.map(_.paramInfoAsSeenFrom(tycon.tpe).bounds) def instantiate(bound: Type, args: List[Type]) = HKTypeLambda.fromParams(tparams, bound).appliedTo(args) - checkBounds(orderedArgs, bounds, instantiate) + if (boundsCheck) checkBounds(orderedArgs, bounds, instantiate) def checkWildcardApply(tp: Type, pos: Position): Unit = tp match { case tp @ AppliedType(tycon, args) if args.exists(_.isInstanceOf[TypeBounds]) =>