From 25a8937f115ed2ac1af33c41c73a621dab4ee712 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 17 Oct 2014 15:28:17 +0200 Subject: [PATCH 01/33] Add echo method to printers. It's often useful to print a value as it is returned. This method avoids the temporary variable to hold the vaue before it is printed and then return. I.e. printer.echo(msg, expr) instead of val x = expr printer.println(msg + x) x --- src/dotty/tools/dotc/config/Printers.scala | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/dotty/tools/dotc/config/Printers.scala b/src/dotty/tools/dotc/config/Printers.scala index 3926be59b121..f8d7f8de557a 100644 --- a/src/dotty/tools/dotc/config/Printers.scala +++ b/src/dotty/tools/dotc/config/Printers.scala @@ -4,10 +4,12 @@ object Printers { class Printer { def println(msg: => String): Unit = System.out.println(msg) + def echo[T](msg: => String, value: T): T = { println(msg + value); value } } object noPrinter extends Printer { override def println(msg: => String): Unit = () + override def echo[T](msg: => String, value: T): T = value } val default: Printer = new Printer From 3fea9472c0d068bc08ae764429ca6b4bca95bcd8 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 17 Oct 2014 15:35:11 +0200 Subject: [PATCH 02/33] Avoid hoisting local classes The patch disables hoisting of classes local to a block into the result type of the block. Instead, we widen the result type of the block to one which reflects all refinements made to the parents type of the local class. Test cases in avoid.scala, t1569.scala. The original t1569.scala no longer works. Why is explained in neg/t1569-failedAvoid.scala --- src/dotty/tools/dotc/typer/TypeAssigner.scala | 16 +++++++++++++++- src/dotty/tools/dotc/typer/Typer.scala | 11 +---------- test/dotc/tests.scala | 1 + tests/neg/t1569-failedAvoid.scala | 9 +++++++++ tests/pos/avoid.scala | 10 ++++++++++ tests/pos/t1569.scala | 5 ----- tests/pos/t1569a.scala | 12 ++++++++++++ 7 files changed, 48 insertions(+), 16 deletions(-) create mode 100644 tests/neg/t1569-failedAvoid.scala create mode 100644 tests/pos/avoid.scala delete mode 100644 tests/pos/t1569.scala create mode 100644 tests/pos/t1569a.scala diff --git a/src/dotty/tools/dotc/typer/TypeAssigner.scala b/src/dotty/tools/dotc/typer/TypeAssigner.scala index ccf67b55b9ae..cb6fefab1472 100644 --- a/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -49,7 +49,21 @@ trait TypeAssigner { case TypeAlias(ref) => apply(ref) case info: ClassInfo => - mapOver(info.instantiatedParents.reduceLeft(ctx.typeComparer.andType(_, _))) + val parentType = info.instantiatedParents.reduceLeft(ctx.typeComparer.andType(_, _)) + def addRefinement(parent: Type, decl: Symbol) = { + val inherited = parentType.findMember(decl.name, info.cls.thisType, Private) + val inheritedInfo = inherited.atSignature(decl.info .signature).info + if (inheritedInfo.exists && decl.info <:< inheritedInfo && !(inheritedInfo <:< decl.info)) + typr.echo( + i"add ref $parent $decl --> ", + RefinedType(parent, decl.name, decl.info)) + else + parent + } + val refinableDecls = info.decls.filterNot( + sym => sym.is(TypeParamAccessor | Private) || sym.isConstructor) + val fullType = (parentType /: refinableDecls)(addRefinement) + mapOver(fullType) case _ => mapOver(tp) } diff --git a/src/dotty/tools/dotc/typer/Typer.scala b/src/dotty/tools/dotc/typer/Typer.scala index 80eb5965c63f..1cab2fa465ac 100644 --- a/src/dotty/tools/dotc/typer/Typer.scala +++ b/src/dotty/tools/dotc/typer/Typer.scala @@ -415,17 +415,8 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit def escapingRefs(block: Block)(implicit ctx: Context): collection.Set[NamedType] = { var hoisted: Set[Symbol] = Set() lazy val locals = localSyms(block.stats).toSet - def isLocal(sym: Symbol): Boolean = - (locals contains sym) && !isHoistableClass(sym) - def isHoistableClass(sym: Symbol) = - sym.isClass && { - (hoisted contains sym) || { - hoisted += sym - !classLeaks(sym.asClass) - } - } def leakingTypes(tp: Type): collection.Set[NamedType] = - tp namedPartsWith (tp => isLocal(tp.symbol)) + tp namedPartsWith (tp => locals.contains(tp.symbol)) def typeLeaks(tp: Type): Boolean = leakingTypes(tp).nonEmpty def classLeaks(sym: ClassSymbol): Boolean = (ctx.owner is Method) || // can't hoist classes out of method bodies diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index 614dc95271db..f93f36ced510 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -99,6 +99,7 @@ class tests extends CompilerTest { @Test def neg_variances = compileFile(negDir, "variances", xerrors = 2) @Test def neg_badAuxConstr = compileFile(negDir, "badAuxConstr", xerrors = 2) @Test def neg_typetest = compileFile(negDir, "typetest", xerrors = 1) + @Test def neg_t1569_failedAvoid = compileFile(negDir, "t1569-failedAvoid", xerrors = 1) @Test def dotc = compileDir(dotcDir + "tools/dotc", twice)(allowDeepSubtypes) @Test def dotc_ast = compileDir(dotcDir + "tools/dotc/ast", twice) @Test def dotc_config = compileDir(dotcDir + "tools/dotc/config", twice) diff --git a/tests/neg/t1569-failedAvoid.scala b/tests/neg/t1569-failedAvoid.scala new file mode 100644 index 000000000000..9d0fbb37a3b8 --- /dev/null +++ b/tests/neg/t1569-failedAvoid.scala @@ -0,0 +1,9 @@ +// This was t1569.scala. +// It fails in dotty because the expected type of the anonymous function in the last line +// is fully determined (C). So that type is taken as the type of the anonymous function. +// See pos/t1569a.scala for related examples that work. +object Bug { + class C { type T } + def foo(x: Int)(y: C)(z: y.T): Unit = {} + foo(3)(new C { type T = String })("hello") +} diff --git a/tests/pos/avoid.scala b/tests/pos/avoid.scala new file mode 100644 index 000000000000..51471feaae65 --- /dev/null +++ b/tests/pos/avoid.scala @@ -0,0 +1,10 @@ +abstract class C { + def y: Any +} + +object test { + val x = new C{ + def y: String = "abc" + } + val z: String = x.y +} diff --git a/tests/pos/t1569.scala b/tests/pos/t1569.scala deleted file mode 100644 index a7200a6d1ebc..000000000000 --- a/tests/pos/t1569.scala +++ /dev/null @@ -1,5 +0,0 @@ -object Bug { - class C { type T } - def foo(x: Int)(y: C)(z: y.T): Unit = {} - foo(3)(new C { type T = String })("hello") -} diff --git a/tests/pos/t1569a.scala b/tests/pos/t1569a.scala new file mode 100644 index 000000000000..6cc3619a4697 --- /dev/null +++ b/tests/pos/t1569a.scala @@ -0,0 +1,12 @@ +object Bug { + class C[T] { type TT = T } + def foo[U](x: Int)(y: C[U])(z: y.TT): Unit = {} + foo(3)(new C[String])("hello") +} + +object Bug2 { + class C { type T } + class D extends C { type T = String } + def foo(x: Int)(y: C)(z: y.T): Unit = {} + foo(3)(new D)("hello") +} From 979fa47ccdbdfc1f495c62b25b95ace9a637a674 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 17 Oct 2014 18:17:40 +0200 Subject: [PATCH 03/33] More assertions in TreeChecker. Flushed out a caching bug in Scopes. --- src/dotty/tools/dotc/core/Scopes.scala | 1 + src/dotty/tools/dotc/transform/TreeChecker.scala | 12 +++++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/dotty/tools/dotc/core/Scopes.scala b/src/dotty/tools/dotc/core/Scopes.scala index 3f7a4cb2ca20..494a26f7e58d 100644 --- a/src/dotty/tools/dotc/core/Scopes.scala +++ b/src/dotty/tools/dotc/core/Scopes.scala @@ -278,6 +278,7 @@ object Scopes { if (e.sym == prev) e.sym = replacement e = lookupNextEntry(e) } + elemsCache = null } /** Lookup a symbol entry matching given name. diff --git a/src/dotty/tools/dotc/transform/TreeChecker.scala b/src/dotty/tools/dotc/transform/TreeChecker.scala index 1b5cc7c07c1a..1448e8bf9033 100644 --- a/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -101,15 +101,17 @@ class TreeChecker { def ownerMatches(symOwner: Symbol, ctxOwner: Symbol): Boolean = symOwner == ctxOwner || ctxOwner.isWeakOwner && ownerMatches(symOwner, ctxOwner.owner) - if(!ownerMatches(tree.symbol.owner, ctx.owner)) { - assert(ownerMatches(tree.symbol.owner, ctx.owner), - i"bad owner; ${tree.symbol} has owner ${tree.symbol.owner}, expected was ${ctx.owner}\n" + - i"owner chain = ${tree.symbol.ownersIterator.toList}%, %, ctxOwners = ${ctx.outersIterator.map(_.owner).toList}%, %") - } + assert(ownerMatches(tree.symbol.owner, ctx.owner), + i"bad owner; ${tree.symbol} has owner ${tree.symbol.owner}, expected was ${ctx.owner}\n" + + i"owner chain = ${tree.symbol.ownersIterator.toList}%, %, ctxOwners = ${ctx.outersIterator.map(_.owner).toList}%, %") } override def typedClassDef(cdef: untpd.TypeDef, cls: ClassSymbol)(implicit ctx: Context) = { val TypeDef(_, _, impl @ Template(constr, _, _, _)) = cdef + assert(cdef.symbol == cls) + assert(impl.symbol.owner == cls) + assert(constr.symbol.owner == cls) + assert(cls.primaryConstructor == constr.symbol, i"mismatch, primary constructor ${cls.primaryConstructor}, in tree = ${constr.symbol}") checkOwner(impl) checkOwner(impl.constr) super.typedClassDef(cdef, cls) From 8b38acbd349a033ba29285397fb54530a25e16e0 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 19 Oct 2014 12:02:02 +0200 Subject: [PATCH 04/33] Insert .package for package object references Package object members are seen as members of the enclosing package during typer. The normalization inserts the missing .package reference to such members. It is necessary to satisfy a new postcondition of FirstTransform: In a selection `x.m`, the type of `x` must derive from the owner of `m`. --- .../tools/dotc/transform/FirstTransform.scala | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/transform/FirstTransform.scala b/src/dotty/tools/dotc/transform/FirstTransform.scala index a8cbb05959b6..9a1bcd7d931f 100644 --- a/src/dotty/tools/dotc/transform/FirstTransform.scala +++ b/src/dotty/tools/dotc/transform/FirstTransform.scala @@ -10,6 +10,8 @@ import Types._ import Constants.Constant import Contexts.Context import Symbols._ +import SymDenotations._ +import Decorators._ import scala.collection.mutable import DenotTransformers._ import typer.Checking @@ -21,6 +23,7 @@ import NameOps._ * - ensures there are companion objects for all classes except module classes * - eliminates some kinds of trees: Imports, NamedArgs, all TypTrees other than TypeTree * - converts Select/Ident/SelectFromTypeTree nodes that refer to types to TypeTrees. + * - inserts `.package` for selections of package object members * - checks the bounds of AppliedTypeTrees * - stubs out native methods */ @@ -29,6 +32,12 @@ class FirstTransform extends MiniPhaseTransform with IdentityDenotTransformer { override def phaseName = "companions" + override def checkPostCondition(tree: Tree)(implicit ctx: Context): Unit = tree match { + case Select(qual, _) if tree.symbol.exists => + assert(qual.tpe derivesFrom tree.symbol.owner, i"non member selection of ${tree.symbol.showLocated} from ${qual.tpe}") + case _ => + } + /** Reorder statements so that module classes always come after their companion classes, add missing companion classes */ private def reorderAndComplete(stats: List[Tree])(implicit ctx: Context): List[Tree] = { val moduleClassDefs, singleClassDefs = mutable.Map[Name, Tree]() @@ -96,7 +105,15 @@ class FirstTransform extends MiniPhaseTransform with IdentityDenotTransformer { } override def transformSelect(tree: Select)(implicit ctx: Context, info: TransformerInfo) = - normalizeType(tree) + normalizeType { + val qual = tree.qualifier + qual.symbol.moduleClass.denot match { + case pkg: PackageClassDenotation if tree.symbol.maybeOwner.isPackageObject => + cpy.Select(tree)(qual select pkg.packageObj.symbol, tree.name) + case _ => + tree + } + } override def transformSelectFromTypeTree(tree: SelectFromTypeTree)(implicit ctx: Context, info: TransformerInfo) = normalizeType(tree) From 0f3a903bebdac5eeaa84b2f8fdd0298bd15468f2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 19 Oct 2014 15:32:30 +0200 Subject: [PATCH 05/33] TreeTypeMap needs to map declarations of mapped classes ... and these mappings have to be part of the applied substitutions. Without the patch, the postCondition of FirstTransform fails for TreeInfo.scala and others, because it selects symbols which are not defined in the mapped class. Unrelated bugfix: JavaArray derives from Object. --- src/dotty/tools/dotc/ast/TreeTypeMap.scala | 51 +++++++++++++++++----- src/dotty/tools/dotc/core/Symbols.scala | 2 +- src/dotty/tools/dotc/core/Types.scala | 2 + 3 files changed, 42 insertions(+), 13 deletions(-) diff --git a/src/dotty/tools/dotc/ast/TreeTypeMap.scala b/src/dotty/tools/dotc/ast/TreeTypeMap.scala index 33be848c1ed1..22428edb6845 100644 --- a/src/dotty/tools/dotc/ast/TreeTypeMap.scala +++ b/src/dotty/tools/dotc/ast/TreeTypeMap.scala @@ -79,14 +79,25 @@ final class TreeTypeMap( override def transform(tree: tpd.Tree)(implicit ctx: Context): tpd.Tree = treeMap(tree) match { case impl @ Template(constr, parents, self, body) => - val tmap = withMappedSyms(impl.symbol :: impl.constr.symbol :: Nil) - val parents1 = parents mapconserve transform - val (_, constr1 :: self1 :: Nil) = transformDefs(constr :: self :: Nil) - val body1 = tmap.transformStats(body) - updateDecls(constr :: body, constr1 :: body1) - cpy.Template(impl)( - constr1.asInstanceOf[DefDef], parents1, self1.asInstanceOf[ValDef], body1) - .withType(tmap.mapType(impl.tpe)) + if (oldOwners contains impl.symbol.owner) { + val tmap = withMappedSyms(localSyms(impl :: self :: Nil)) + cpy.Template(impl)( + constr = tmap.transformSub(constr), + parents = parents mapconserve transform, + self = tmap.transformSub(self), + body = body mapconserve tmap.transform + ).withType(tmap.mapType(impl.tpe)) + } + else { + val tmap = withMappedSyms(impl.symbol :: impl.constr.symbol :: Nil) + val parents1 = parents mapconserve transform + val (_, constr1 :: self1 :: Nil) = transformDefs(constr :: self :: Nil) + val body1 = tmap.transformStats(body) + updateDecls(constr :: body, constr1 :: body1) + cpy.Template(impl)( + constr1.asInstanceOf[DefDef], parents1, self1.asInstanceOf[ValDef], body1) + .withType(tmap.mapType(impl.tpe)) + } case tree1 => tree1.withType(mapType(tree1.tpe)) match { case id: Ident if tpd.needsSelect(id.tpe) => @@ -160,8 +171,24 @@ final class TreeTypeMap( * and return a treemap that contains the substitution * between original and mapped symbols. */ - def withMappedSyms(syms: List[Symbol]): TreeTypeMap = { - val mapped = ctx.mapSymbols(syms, this) - withSubstitution(syms, mapped) - } + def withMappedSyms(syms: List[Symbol]): TreeTypeMap = + withMappedSyms(syms, ctx.mapSymbols(syms, this)) + + /** The tree map with the substitution between originals `syms` + * and mapped symbols `mapped`. Also goes into mapped classes + * and substitutes their declarations. + */ + def withMappedSyms(syms: List[Symbol], mapped: List[Symbol]): TreeTypeMap = + if (mapped eq syms) this + else { + val substMap = withSubstitution(syms, mapped) + val mappedClasses = mapped.filter(_.isClass) + (substMap /: mappedClasses) { (tmap, cls) => + val origDcls = cls.decls.toList + val mappedDcls = ctx.mapSymbols(origDcls, tmap) + val tmap1 = tmap.withMappedSyms(origDcls, mappedDcls) + (origDcls, mappedDcls).zipped.foreach(cls.asClass.replace) + tmap1 + } + } } diff --git a/src/dotty/tools/dotc/core/Symbols.scala b/src/dotty/tools/dotc/core/Symbols.scala index 0174f75cf6f6..cedc0adc899b 100644 --- a/src/dotty/tools/dotc/core/Symbols.scala +++ b/src/dotty/tools/dotc/core/Symbols.scala @@ -267,7 +267,7 @@ trait Symbols { this: Context => * Cross symbol references are brought over from originals to copies. * Do not copy any symbols if all attributes of all symbols stay the same. */ - def mapSymbols(originals: List[Symbol], ttmap: TreeTypeMap) = + def mapSymbols(originals: List[Symbol], ttmap: TreeTypeMap): List[Symbol] = if (originals forall (sym => (ttmap.mapType(sym.info) eq sym.info) && !(ttmap.oldOwners contains sym.owner))) diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala index d93e4eb09338..33ce71ad344e 100644 --- a/src/dotty/tools/dotc/core/Types.scala +++ b/src/dotty/tools/dotc/core/Types.scala @@ -116,6 +116,8 @@ object Types { tp.tp1.derivesFrom(cls) || tp.tp2.derivesFrom(cls) case tp: OrType => tp.tp1.derivesFrom(cls) && tp.tp2.derivesFrom(cls) + case tp: JavaArrayType => + cls == defn.ObjectClass case _ => false } From 1a81244b68e58bde6a3d03551f1d92f15c3ff719 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 19 Oct 2014 16:57:14 +0200 Subject: [PATCH 06/33] Fix to TreeTypeMap Now handles the case where a class symbol itself is not changed by the map, but one of its declarations is. In this case we need to back out, and create new symbols for the class and all other symbols that are defined in the same scope as the class. --- src/dotty/tools/dotc/ast/TreeTypeMap.scala | 44 ++++++++-------------- src/dotty/tools/dotc/core/Symbols.scala | 6 +-- 2 files changed, 19 insertions(+), 31 deletions(-) diff --git a/src/dotty/tools/dotc/ast/TreeTypeMap.scala b/src/dotty/tools/dotc/ast/TreeTypeMap.scala index 22428edb6845..32c89c94fb51 100644 --- a/src/dotty/tools/dotc/ast/TreeTypeMap.scala +++ b/src/dotty/tools/dotc/ast/TreeTypeMap.scala @@ -79,25 +79,13 @@ final class TreeTypeMap( override def transform(tree: tpd.Tree)(implicit ctx: Context): tpd.Tree = treeMap(tree) match { case impl @ Template(constr, parents, self, body) => - if (oldOwners contains impl.symbol.owner) { - val tmap = withMappedSyms(localSyms(impl :: self :: Nil)) - cpy.Template(impl)( + val tmap = withMappedSyms(localSyms(impl :: self :: Nil)) + cpy.Template(impl)( constr = tmap.transformSub(constr), parents = parents mapconserve transform, self = tmap.transformSub(self), body = body mapconserve tmap.transform ).withType(tmap.mapType(impl.tpe)) - } - else { - val tmap = withMappedSyms(impl.symbol :: impl.constr.symbol :: Nil) - val parents1 = parents mapconserve transform - val (_, constr1 :: self1 :: Nil) = transformDefs(constr :: self :: Nil) - val body1 = tmap.transformStats(body) - updateDecls(constr :: body, constr1 :: body1) - cpy.Template(impl)( - constr1.asInstanceOf[DefDef], parents1, self1.asInstanceOf[ValDef], body1) - .withType(tmap.mapType(impl.tpe)) - } case tree1 => tree1.withType(mapType(tree1.tpe)) match { case id: Ident if tpd.needsSelect(id.tpe) => @@ -171,24 +159,24 @@ final class TreeTypeMap( * and return a treemap that contains the substitution * between original and mapped symbols. */ - def withMappedSyms(syms: List[Symbol]): TreeTypeMap = - withMappedSyms(syms, ctx.mapSymbols(syms, this)) + def withMappedSyms(syms: List[Symbol], mapAlways: Boolean = false): TreeTypeMap = + withMappedSyms(syms, ctx.mapSymbols(syms, this, mapAlways)) /** The tree map with the substitution between originals `syms` * and mapped symbols `mapped`. Also goes into mapped classes * and substitutes their declarations. */ - def withMappedSyms(syms: List[Symbol], mapped: List[Symbol]): TreeTypeMap = - if (mapped eq syms) this - else { - val substMap = withSubstitution(syms, mapped) - val mappedClasses = mapped.filter(_.isClass) - (substMap /: mappedClasses) { (tmap, cls) => - val origDcls = cls.decls.toList - val mappedDcls = ctx.mapSymbols(origDcls, tmap) - val tmap1 = tmap.withMappedSyms(origDcls, mappedDcls) - (origDcls, mappedDcls).zipped.foreach(cls.asClass.replace) - tmap1 - } + def withMappedSyms(syms: List[Symbol], mapped: List[Symbol]): TreeTypeMap = { + val symsChanged = syms ne mapped + val substMap = withSubstitution(syms, mapped) + val fullMap = (substMap /: mapped.filter(_.isClass)) { (tmap, cls) => + val origDcls = cls.decls.toList + val mappedDcls = ctx.mapSymbols(origDcls, tmap) + val tmap1 = tmap.withMappedSyms(origDcls, mappedDcls) + if (symsChanged) (origDcls, mappedDcls).zipped.foreach(cls.asClass.replace) + tmap1 } + if (symsChanged || (fullMap eq substMap)) fullMap + else withMappedSyms(syms, mapAlways = true) + } } diff --git a/src/dotty/tools/dotc/core/Symbols.scala b/src/dotty/tools/dotc/core/Symbols.scala index cedc0adc899b..83fb2c134e91 100644 --- a/src/dotty/tools/dotc/core/Symbols.scala +++ b/src/dotty/tools/dotc/core/Symbols.scala @@ -267,10 +267,10 @@ trait Symbols { this: Context => * Cross symbol references are brought over from originals to copies. * Do not copy any symbols if all attributes of all symbols stay the same. */ - def mapSymbols(originals: List[Symbol], ttmap: TreeTypeMap): List[Symbol] = - if (originals forall (sym => + def mapSymbols(originals: List[Symbol], ttmap: TreeTypeMap, mapAlways: Boolean = false): List[Symbol] = + if (originals.forall(sym => (ttmap.mapType(sym.info) eq sym.info) && - !(ttmap.oldOwners contains sym.owner))) + !(ttmap.oldOwners contains sym.owner)) && !mapAlways) originals else { val copies: List[Symbol] = for (original <- originals) yield From 7167c225841a8261086e42e3e8721a4059a484bb Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 19 Oct 2014 18:43:07 +0200 Subject: [PATCH 07/33] Implement findMember for JavaArrays ... by forwarding to Object. Without this, LambdaLift fails for core/Contexts.scala. --- src/dotty/tools/dotc/core/Types.scala | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala index 33ce71ad344e..d0ddfdd28b50 100644 --- a/src/dotty/tools/dotc/core/Types.scala +++ b/src/dotty/tools/dotc/core/Types.scala @@ -410,6 +410,8 @@ object Types { goAnd(l, r) case OrType(l, r) => goOr(l, r) + case tp: JavaArrayType => + defn.ObjectType.findMember(name, pre, excluded) case ErrorType => ctx.newErrorSymbol(pre.classSymbol orElse defn.RootClass, name) case _ => From f590cb3564e47f212ba0b8c6d69c2d0f86108de9 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 21 Oct 2014 16:32:08 +0200 Subject: [PATCH 08/33] Rename flag Static -> JavaStatic Static has two meanings: In the Java sense, and in the Scala sense, where it means a member of a package or static module. The change makes it clear that the flag means Static in the Java sense. --- src/dotty/tools/dotc/core/Flags.scala | 14 ++++++++------ src/dotty/tools/dotc/core/SymDenotations.scala | 2 +- .../dotc/core/pickling/ClassfileConstants.scala | 2 +- .../tools/dotc/core/pickling/PickleBuffer.scala | 2 +- src/dotty/tools/dotc/transform/LambdaLift.scala | 2 +- 5 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/dotty/tools/dotc/core/Flags.scala b/src/dotty/tools/dotc/core/Flags.scala index a27dd6614fac..e5bf27eaeb00 100644 --- a/src/dotty/tools/dotc/core/Flags.scala +++ b/src/dotty/tools/dotc/core/Flags.scala @@ -331,7 +331,9 @@ object Flags { final val JavaDefined = commonFlag(30, "") /** Symbol is implemented as a Java static */ - final val Static = commonFlag(31, "") + final val JavaStatic = commonFlag(31, "") + final val JavaStaticTerm = JavaStatic.toTermFlags + final val JavaStaticType = JavaStatic.toTypeFlags /** Variable is accessed from nested function. */ final val Captured = termFlag(32, "") @@ -421,7 +423,7 @@ object Flags { /** Flags representing source modifiers */ final val SourceModifierFlags = commonFlags(Private, Protected, Abstract, Final, - Sealed, Case, Implicit, Override, AbsOverride, Lazy, Static) + Sealed, Case, Implicit, Override, AbsOverride, Lazy, JavaStatic) /** Flags representing modifiers that can appear in trees */ final val ModifierFlags = @@ -436,7 +438,7 @@ object Flags { /** Flags guaranteed to be set upon symbol creation */ final val FromStartFlags = AccessFlags | Module | Package | Deferred | MethodOrHKCommon | Param | ParamAccessor | Scala2ExistentialCommon | - InSuperCall | Touched | Static | CovariantOrOuter | ContravariantOrLabel | ExpandedName | AccessorOrSealed | + InSuperCall | Touched | JavaStatic | CovariantOrOuter | ContravariantOrLabel | ExpandedName | AccessorOrSealed | CaseAccessorOrTypeArgument | Fresh | Frozen | Erroneous | ImplicitCommon | Permanent | SelfNameOrImplClass @@ -473,7 +475,7 @@ object Flags { */ final val RetainedModuleValAndClassFlags: FlagSet = AccessFlags | Package | Case | - Synthetic | ExpandedName | JavaDefined | Static | Artifact | + Synthetic | ExpandedName | JavaDefined | JavaStatic | Artifact | Erroneous | Lifted | MixedIn | Specialized /** Flags that can apply to a module val */ @@ -487,7 +489,7 @@ object Flags { /** Packages and package classes always have these flags set */ final val PackageCreationFlags = - Module | Package | Final | JavaDefined | Static + Module | Package | Final | JavaDefined /** These flags are pickled */ final val PickledFlags = flagRange(FirstFlag, FirstNotPickledFlag) @@ -562,7 +564,7 @@ object Flags { final val ProtectedLocal = allOf(Protected, Local) /** Java symbol which is `protected` and `static` */ - final val StaticProtected = allOf(JavaDefined, Protected, Static) + final val StaticProtected = allOf(JavaDefined, Protected, JavaStatic) final val AbstractFinal = allOf(Abstract, Final) final val AbstractSealed = allOf(Abstract, Sealed) diff --git a/src/dotty/tools/dotc/core/SymDenotations.scala b/src/dotty/tools/dotc/core/SymDenotations.scala index ae37ab87cdeb..9a5be70d1980 100644 --- a/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/src/dotty/tools/dotc/core/SymDenotations.scala @@ -390,7 +390,7 @@ object SymDenotations { /** Is this denotation static (i.e. with no outer instance)? */ final def isStatic(implicit ctx: Context) = - (this is Static) || this.exists && owner.isStaticOwner + (this is JavaStatic) || this.exists && owner.isStaticOwner /** Is this a package class or module class that defines static symbols? */ final def isStaticOwner(implicit ctx: Context): Boolean = diff --git a/src/dotty/tools/dotc/core/pickling/ClassfileConstants.scala b/src/dotty/tools/dotc/core/pickling/ClassfileConstants.scala index c3850d0fd76d..c35b9ca474fd 100644 --- a/src/dotty/tools/dotc/core/pickling/ClassfileConstants.scala +++ b/src/dotty/tools/dotc/core/pickling/ClassfileConstants.scala @@ -343,7 +343,7 @@ object ClassfileConstants { case JAVA_ACC_PROTECTED => Protected case JAVA_ACC_FINAL => Final case JAVA_ACC_SYNTHETIC => Synthetic - case JAVA_ACC_STATIC => Static + case JAVA_ACC_STATIC => JavaStatic case JAVA_ACC_ABSTRACT => if (isAnnotation) EmptyFlags else if (isClass) Abstract else Deferred case JAVA_ACC_INTERFACE => if (isAnnotation) EmptyFlags else JavaInterface case _ => EmptyFlags diff --git a/src/dotty/tools/dotc/core/pickling/PickleBuffer.scala b/src/dotty/tools/dotc/core/pickling/PickleBuffer.scala index ef2b4acb2700..d2a05bf3a4dc 100644 --- a/src/dotty/tools/dotc/core/pickling/PickleBuffer.scala +++ b/src/dotty/tools/dotc/core/pickling/PickleBuffer.scala @@ -236,7 +236,7 @@ object PickleBuffer { JAVA -> JavaDefined, SYNTHETIC -> Synthetic, STABLE -> Stable, - STATIC -> Static, + STATIC -> JavaStatic, CASEACCESSOR -> CaseAccessor, DEFAULTPARAM -> (DefaultParameterized, Trait), BRIDGE -> Bridge, diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index a08df1c33ef0..4a59be45a393 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -257,7 +257,7 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this private def liftLocals()(implicit ctx: Context): Unit = { for ((local, lOwner) <- liftedOwner) { val (newOwner, maybeStatic) = - if (lOwner is Package) (local.topLevelClass, Static) + if (lOwner is Package) (local.topLevelClass, JavaStatic) else (lOwner, EmptyFlags) val maybeNotJavaPrivate = if (calledFromInner(local)) NotJavaPrivate else EmptyFlags local.copySymDenotation( From e3b0fa2ede56393165759eecd7bc916d24835ee1 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 21 Oct 2014 16:36:10 +0200 Subject: [PATCH 09/33] Fix flatten problem in erasure Statement sequences need to be flattened after processing, not before. --- src/dotty/tools/dotc/transform/Erasure.scala | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/dotty/tools/dotc/transform/Erasure.scala b/src/dotty/tools/dotc/transform/Erasure.scala index ed853f8f1c2c..198e9b621f38 100644 --- a/src/dotty/tools/dotc/transform/Erasure.scala +++ b/src/dotty/tools/dotc/transform/Erasure.scala @@ -183,6 +183,7 @@ object Erasure extends TypeTestsCasts{ } /** Generate a synthetic cast operation from tree.tpe to pt. + * Does not do any boxing/unboxing (this is handled upstream). */ def cast(tree: Tree, pt: Type)(implicit ctx: Context): Tree = { // TODO: The commented out assertion fails for tailcall/t6574.scala @@ -315,7 +316,7 @@ object Erasure extends TypeTestsCasts{ override def typedThis(tree: untpd.This)(implicit ctx: Context): Tree = if (tree.symbol == ctx.owner.enclosingClass || tree.symbol.isStaticOwner) promote(tree) else { - ctx.log(i"computing outer path from ${ctx.owner.ownersIterator.toList}%, % to ${tree.symbol}") + ctx.log(i"computing outer path from ${ctx.owner.ownersIterator.toList}%, % to ${tree.symbol}, encl class = ${ctx.owner.enclosingClass}") outer.path(tree.symbol) } @@ -377,10 +378,8 @@ object Erasure extends TypeTestsCasts{ EmptyTree override def typedStats(stats: List[untpd.Tree], exprOwner: Symbol)(implicit ctx: Context): List[Tree] = { - val statsFlatten = Trees.flatten(stats) - val stats1 = super.typedStats(statsFlatten, exprOwner) - - if (ctx.owner.isClass) stats1:::addBridges(statsFlatten, stats1)(ctx) else stats1 + val stats1 = Trees.flatten(super.typedStats(stats, exprOwner)) + if (ctx.owner.isClass) stats1 ::: addBridges(stats, stats1)(ctx) else stats1 } // this implementation doesn't check for bridge clashes with value types! From 04001befb1a7f08da0c38166eed61322104adbaf Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 21 Oct 2014 16:38:27 +0200 Subject: [PATCH 10/33] Two fixes to avoid scanning package contents typeParams and outerAccessor both potentially scan all declarations of a class. The fixes make sure this is never done for packages. --- src/dotty/tools/dotc/core/SymDenotations.scala | 2 +- src/dotty/tools/dotc/transform/ExplicitOuter.scala | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/dotty/tools/dotc/core/SymDenotations.scala b/src/dotty/tools/dotc/core/SymDenotations.scala index 9a5be70d1980..4cc15897c200 100644 --- a/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/src/dotty/tools/dotc/core/SymDenotations.scala @@ -976,7 +976,7 @@ object SymDenotations { /** The type parameters of this class */ override final def typeParams(implicit ctx: Context): List[TypeSymbol] = { def computeTypeParams = { - if (ctx.erasedTypes && (symbol ne defn.ArrayClass)) Nil + if (ctx.erasedTypes || is(Module)) Nil // fast return for modules to avoid scanning package decls else if (this ne initial) initial.asSymDenotation.typeParams else decls.filter(sym => (sym is TypeParam) && sym.owner == symbol).asInstanceOf[List[TypeSymbol]] diff --git a/src/dotty/tools/dotc/transform/ExplicitOuter.scala b/src/dotty/tools/dotc/transform/ExplicitOuter.scala index 179f8d7124e6..d056d7e351a2 100644 --- a/src/dotty/tools/dotc/transform/ExplicitOuter.scala +++ b/src/dotty/tools/dotc/transform/ExplicitOuter.scala @@ -179,7 +179,8 @@ object ExplicitOuter { * definitions in the class to find the one with the OuterAccessor flag. */ def outerAccessor(cls: ClassSymbol)(implicit ctx: Context): Symbol = - cls.info.member(outerAccName(cls)).suchThat(_ is OuterAccessor).symbol orElse + if (cls.isStatic) NoSymbol // fast return to avoid scanning package decls + else cls.info.member(outerAccName(cls)).suchThat(_ is OuterAccessor).symbol orElse cls.info.decls.find(_ is OuterAccessor).getOrElse(NoSymbol) /** Class has an outer accessor. Can be called only after phase ExplicitOuter. */ From 98deca5e3e5e98c77b1440c8ab0d9bfd232e7357 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 21 Oct 2014 16:40:09 +0200 Subject: [PATCH 11/33] Fix to enclosingClass The skip logic in enclosing class worked only when the original symbol was labelled inSuperCall. The patch makes it work also for symbols that are in turn owned by an inSuperCall symbol. Also it treats JavaStatic terms as also not being enclosed by the lexically enclosing class. --- src/dotty/tools/dotc/core/SymDenotations.scala | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/dotty/tools/dotc/core/SymDenotations.scala b/src/dotty/tools/dotc/core/SymDenotations.scala index 4cc15897c200..ae8fceeb743b 100644 --- a/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/src/dotty/tools/dotc/core/SymDenotations.scala @@ -666,10 +666,16 @@ object SymDenotations { * for these definitions. */ final def enclosingClass(implicit ctx: Context): Symbol = { - def enclClass(d: SymDenotation): Symbol = - if (d.isClass || !d.exists) d.symbol else enclClass(d.owner) - val cls = enclClass(this) - if (this is InSuperCall) cls.owner.enclosingClass else cls + def enclClass(sym: Symbol, skip: Boolean): Symbol = { + def newSkip = sym.is(InSuperCall) || sym.is(JavaStaticTerm) + if (!sym.exists) + NoSymbol + else if (sym.isClass) + if (skip) enclClass(sym.owner, newSkip) else sym + else + enclClass(sym.owner, skip || newSkip) + } + enclClass(symbol, false) } final def isEffectivelyFinal(implicit ctx: Context): Boolean = { From 138045cc5545519c87044147dd7bb5b14729d2d2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 21 Oct 2014 16:41:16 +0200 Subject: [PATCH 12/33] Fixes to LambdaLift Several fixes to LambdaLift. The test suite now succeeds with LambdaLift enabled. --- src/dotty/tools/dotc/Compiler.scala | 4 ++-- .../tools/dotc/transform/LambdaLift.scala | 23 ++++++++++++------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/dotty/tools/dotc/Compiler.scala b/src/dotty/tools/dotc/Compiler.scala index 1aa1cce1073d..ec899ebb8716 100644 --- a/src/dotty/tools/dotc/Compiler.scala +++ b/src/dotty/tools/dotc/Compiler.scala @@ -53,8 +53,8 @@ class Compiler { new Literalize, new GettersSetters), List(new Erasure), - List(new CapturedVars, new Constructors)/*, - List(new LambdaLift)*/ + List(new CapturedVars, new Constructors), + List(new LambdaLift) ) var runId = 1 diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index 4a59be45a393..cd74da603510 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -88,7 +88,7 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this } def narrowLiftedOwner(sym: Symbol, owner: Symbol)(implicit ctx: Context) = { - println(i"narrow lifted $sym") + println(i"narrow lifted $sym to $owner") if (sym.owner.skipConstructor.isTerm && owner.isProperlyContainedIn(liftedOwner(sym))) { changedLiftedOwner = true @@ -183,7 +183,7 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this case tree: TypeDef => if (sym.owner.isTerm) liftedOwner(sym) = sym.topLevelClass.owner case tree: Template => - liftedDefs(enclosure) = new mutable.ListBuffer + liftedDefs(tree.symbol.owner) = new mutable.ListBuffer case _ => } foldOver(enclosure, tree) @@ -292,8 +292,11 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this private def proxy(sym: Symbol)(implicit ctx: Context): Symbol = { def searchIn(enclosure: Symbol): Symbol = { - if (!enclosure.exists) - throw new IllegalArgumentException(i"Could not find proxy for ${sym.showDcl} in ${sym.ownersIterator.toList}, currentOwner= $currentEnclosure") + if (!enclosure.exists) { + def enclosures(encl: Symbol): List[Symbol] = + if (encl.exists) encl :: enclosures(encl.enclosure) else Nil + throw new IllegalArgumentException(i"Could not find proxy for ${sym.showDcl} in ${sym.ownersIterator.toList}, encl = $currentEnclosure, owners = ${currentEnclosure.ownersIterator.toList}%, %; enclosures = ${enclosures(currentEnclosure)}%, %") + } ctx.debuglog(i"searching for $sym(${sym.owner}) in $enclosure") proxyMap get enclosure match { case Some(pmap) => @@ -309,13 +312,15 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this } private def memberRef(sym: Symbol)(implicit ctx: Context, info: TransformerInfo): Tree = { - val clazz = sym.owner - val qual = if (clazz.isStaticOwner) singleton(clazz.thisType) else outer.path(clazz) + val clazz = sym.enclosingClass + val qual = + if (clazz.isStaticOwner) singleton(clazz.thisType) + else outer(ctx.withPhase(thisTransform)).path(clazz) transformFollowingDeep(qual.select(sym)) } private def proxyRef(sym: Symbol)(implicit ctx: Context, info: TransformerInfo): Tree = { - val psym = proxy(sym) + val psym = proxy(sym)(ctx.withPhase(thisTransform)) transformFollowingDeep(if (psym.owner.isTerm) ref(psym) else memberRef(psym)) } @@ -368,7 +373,9 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this val sym = tree.symbol val proxyHolder = sym.skipConstructor if (needsLifting(proxyHolder)) { - val paramsAdded = addFreeParams(tree, proxies(proxyHolder)).asInstanceOf[DefDef] + var paramsAdded = addFreeParams(tree, proxies(proxyHolder)).asInstanceOf[DefDef] + if (sym.is(JavaStatic) && !paramsAdded.mods.is(JavaStatic)) + paramsAdded = cpy.DefDef(paramsAdded)(mods = paramsAdded.mods | JavaStatic) if (sym.isConstructor) paramsAdded else liftDef(paramsAdded) } else tree From e7cc8a4012b9bfc9bc6ae811b37cc468ced46ff2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 22 Oct 2014 11:45:34 +0200 Subject: [PATCH 13/33] Added missing case for SuperTypes to TypeComparer --- src/dotty/tools/dotc/core/TypeComparer.scala | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/dotty/tools/dotc/core/TypeComparer.scala b/src/dotty/tools/dotc/core/TypeComparer.scala index f36572755eb8..dcd8af3b65e4 100644 --- a/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/src/dotty/tools/dotc/core/TypeComparer.scala @@ -515,6 +515,14 @@ class TypeComparer(initctx: Context) extends DotClass { case _ => secondTry(tp1, tp2) } + case tp2: SuperType => + tp1 match { + case tp1: SuperType => + isSubType(tp1.thistpe, tp2.thistpe) && + isSameType(tp1.supertpe, tp2.supertpe) + case _ => + secondTry(tp1, tp2) + } case AndType(tp21, tp22) => isSubType(tp1, tp21) && isSubType(tp1, tp22) case ErrorType => From 854373753526248737ee554290b73fa583f0c264 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 22 Oct 2014 11:46:03 +0200 Subject: [PATCH 14/33] SuperTypes are now promoted in Retyper; lambdaLift fails to Ycheck otherwise. --- src/dotty/tools/dotc/typer/ReTyper.scala | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/dotty/tools/dotc/typer/ReTyper.scala b/src/dotty/tools/dotc/typer/ReTyper.scala index 6895f997874a..f36e8e2fd10d 100644 --- a/src/dotty/tools/dotc/typer/ReTyper.scala +++ b/src/dotty/tools/dotc/typer/ReTyper.scala @@ -47,6 +47,9 @@ class ReTyper extends Typer { override def typedThis(tree: untpd.This)(implicit ctx: Context): Tree = promote(tree) + override def typedSuper(tree: untpd.Super, pt: Type)(implicit ctx: Context): Tree = + promote(tree) + override def typedTypeTree(tree: untpd.TypeTree, pt: Type)(implicit ctx: Context): TypeTree = promote(tree) From 651ff01e07b90c481e30afdd1500f617d74aeeb4 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 22 Oct 2014 11:47:43 +0200 Subject: [PATCH 15/33] LambdaLift checks now explicitly for references to labels outside scope. Nested methods cannot refer to labels in theior environment. Needs a fix in TailCalls. Moved failing test to pending. --- src/dotty/tools/dotc/transform/LambdaLift.scala | 10 +++++++--- tests/{ => pending}/pos/tailcall/t1672.scala | 0 2 files changed, 7 insertions(+), 3 deletions(-) rename tests/{ => pending}/pos/tailcall/t1672.scala (100%) diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index cd74da603510..f34b5479a7e3 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -31,7 +31,7 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this import ast.tpd._ /** the following two members override abstract members in Transform */ - val phaseName: String = "lambdalift" + val phaseName: String = "lambdaLift" override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[Constructors]) // Constructors has to happen before LambdaLift because the lambda lift logic @@ -164,9 +164,13 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this val sym = tree.symbol tree match { case tree: Ident => - if (sym.maybeOwner.isTerm) - if (sym is (Method, butNot = Label)) markCalled(sym, enclosure) + if (sym.maybeOwner.isTerm) { + if (sym is Label) + assert(enclosure == sym.enclosure, + i"attempt to refer to label $sym from nested $enclosure") + else if (sym is Method) markCalled(sym, enclosure) else if (sym.isTerm) markFree(sym, enclosure) + } case tree: Select => if (sym.isConstructor && sym.owner.owner.isTerm) markCalled(sym, enclosure) diff --git a/tests/pos/tailcall/t1672.scala b/tests/pending/pos/tailcall/t1672.scala similarity index 100% rename from tests/pos/tailcall/t1672.scala rename to tests/pending/pos/tailcall/t1672.scala From a426e9280aa7ed3dbad923a8b2110b6a1a281771 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 24 Oct 2014 18:13:13 +0200 Subject: [PATCH 16/33] Fixes erasure of super - supertype components needs to be recursively erased Without this fix, some files do not pass -Ycheck:lambdaLift --- src/dotty/tools/dotc/TypeErasure.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/TypeErasure.scala b/src/dotty/tools/dotc/TypeErasure.scala index 50aaafc82279..851be76583bf 100644 --- a/src/dotty/tools/dotc/TypeErasure.scala +++ b/src/dotty/tools/dotc/TypeErasure.scala @@ -258,8 +258,10 @@ class TypeErasure(isJava: Boolean, isSemi: Boolean, isConstructor: Boolean, wild else this(parent) case tp: TermRef => this(tp.widen) - case ThisType(_) | SuperType(_, _) => + case ThisType(_) => tp + case SuperType(thistpe, supertpe) => + SuperType(this(thistpe), this(supertpe)) case ExprType(rt) => MethodType(Nil, Nil, this(rt)) case tp: TypeProxy => From 17b78ba1f82824865f0371c168050c94bf5b3720 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 24 Oct 2014 18:21:47 +0200 Subject: [PATCH 17/33] Strenghten postCondition of firstTransform All tests pass, but good to have the condition in there. --- src/dotty/tools/dotc/transform/FirstTransform.scala | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/dotty/tools/dotc/transform/FirstTransform.scala b/src/dotty/tools/dotc/transform/FirstTransform.scala index 9a1bcd7d931f..05255c982a93 100644 --- a/src/dotty/tools/dotc/transform/FirstTransform.scala +++ b/src/dotty/tools/dotc/transform/FirstTransform.scala @@ -35,6 +35,9 @@ class FirstTransform extends MiniPhaseTransform with IdentityDenotTransformer { override def checkPostCondition(tree: Tree)(implicit ctx: Context): Unit = tree match { case Select(qual, _) if tree.symbol.exists => assert(qual.tpe derivesFrom tree.symbol.owner, i"non member selection of ${tree.symbol.showLocated} from ${qual.tpe}") + case _: TypeTree => + case _: Import | _: NamedArg | _: TypTree => + assert(false, i"illegal tree: $tree") case _ => } From 02afa11c809798bc218499ae26050c426d36f42c Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 24 Oct 2014 18:23:29 +0200 Subject: [PATCH 18/33] Initialize lambda lift maps ... so that they do not spill over between compilation units. It would be better to wipe the maps after processing a compilation unit, but right now we do not have a hook for that. A better solution should be possible once we replace init by "prepareUnit/transformUnit". --- src/dotty/tools/dotc/transform/LambdaLift.scala | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index f34b5479a7e3..a285685947f2 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -280,6 +280,12 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this } override def init(implicit ctx: Context, info: TransformerInfo) = { + free.clear() + proxyMap.clear() + called.clear() + calledFromInner.clear() + liftedOwner.clear() + liftedDefs.clear() assert(ctx.phase == thisTransform) (new CollectDependencies).traverse(NoSymbol, ctx.compilationUnit.tpdTree) computeFreeVars() From 55715f12a08646f54f324c5387c38615c433c4d1 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 24 Oct 2014 18:25:24 +0200 Subject: [PATCH 19/33] Replace some idents by selects in LambdaLift If a symbol becomes a class field, references to it need to be selections, or else we get a "bad type" assertion violation in TreeChecker. --- src/dotty/tools/dotc/transform/LambdaLift.scala | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index a285685947f2..47a61a5436a7 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -366,8 +366,11 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this override def transformIdent(tree: Ident)(implicit ctx: Context, info: TransformerInfo) = { val sym = tree.symbol tree.tpe match { - case TermRef(NoPrefix, _) if sym.enclosure != currentEnclosure && !sym.isStatic => - (if (sym is Method) memberRef(sym) else proxyRef(sym)).withPos(tree.pos) + case tpe @ TermRef(prefix, _) => + if ((prefix eq NoPrefix) && sym.enclosure != currentEnclosure && !sym.isStatic) + (if (sym is Method) memberRef(sym) else proxyRef(sym)).withPos(tree.pos) + else if (!prefixIsElidable(tpe)) ref(tpe) + else tree case _ => tree } From 4d370b6073bec9706d427f82c4a6a40fa22fe6d0 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 24 Oct 2014 18:29:57 +0200 Subject: [PATCH 20/33] Almost all tests pass -Ycheck:lambdLift Only exception: dotc/transform. This seems to be for two reasons: 1) The call-by-name functions used in Decorator#foldRightBN cannot be translated correctly at their use points. 2) An anonymous function in Constructors are not correctly lifted. 2) could be related to missing/duplicated symbols in pattern matcher. I'll follow up with a commit that points these out. --- src/dotty/tools/dotc/transform/Erasure.scala | 2 +- test/dotc/tests.scala | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/dotty/tools/dotc/transform/Erasure.scala b/src/dotty/tools/dotc/transform/Erasure.scala index 198e9b621f38..31397e08a723 100644 --- a/src/dotty/tools/dotc/transform/Erasure.scala +++ b/src/dotty/tools/dotc/transform/Erasure.scala @@ -101,7 +101,7 @@ class Erasure extends Phase with DenotTransformer { thisTransformer => } def assertErased(tp: Type, tree: tpd.Tree = tpd.EmptyTree)(implicit ctx: Context): Unit = - assert(isErasedType(tp), i"The type $tp - ${tp.toString} of class ${tp.getClass} of tree $tree / ${tree.getClass} is illegal after erasure, phase = ${ctx.phase}") + assert(isErasedType(tp), i"The type $tp - ${tp.toString} of class ${tp.getClass} of tree $tree : ${tree.tpe} / ${tree.getClass} is illegal after erasure, phase = ${ctx.phase}") } object Erasure extends TypeTestsCasts{ diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index f93f36ced510..901167a5649c 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -15,7 +15,7 @@ class tests extends CompilerTest { implicit val defaultOptions = noCheckOptions ++ List( "-Yno-deep-subtypes", - "-Ycheck:patternMatcher,gettersSetters,constructors" + "-Ycheck:patternMatcher,gettersSetters,lambdaLift" ) val twice = List("#runs", "2", "-YnoDoubleBindings") @@ -105,7 +105,10 @@ class tests extends CompilerTest { @Test def dotc_config = compileDir(dotcDir + "tools/dotc/config", twice) @Test def dotc_core = compileDir(dotcDir + "tools/dotc/core", twice)(allowDeepSubtypes) @Test def dotc_core_pickling = compileDir(dotcDir + "tools/dotc/core/pickling", twice)(allowDeepSubtypes) - @Test def dotc_transform = compileDir(dotcDir + "tools/dotc/transform", twice) + + //@Test def dotc_transform = compileDir(dotcDir + "tools/dotc/transform", twice) + //disabled, awaiting fix for call-by-name function types. + @Test def dotc_parsing = compileDir(dotcDir + "tools/dotc/parsing", twice) @Test def dotc_printing = compileDir(dotcDir + "tools/dotc/printing", twice) @Test def dotc_reporting = compileDir(dotcDir + "tools/dotc/reporting", twice) From 3a250720a0833d42ed6b23b5837de64b6ce34aed Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 24 Oct 2014 18:32:48 +0200 Subject: [PATCH 21/33] Add missing and double symbol checking to TreeChecker TreeChecker now tests that a symbol does not have two definitions that define it, and that every reference to a symbol owner by a term is in the scope of a definition of that symbol. Both tests fail on several files for pattern matcher. --- src/dotty/tools/dotc/ast/tpd.scala | 7 +++ src/dotty/tools/dotc/core/Decorators.scala | 5 ++ .../tools/dotc/transform/TreeChecker.scala | 46 ++++++++++++++++++- src/dotty/tools/dotc/typer/Typer.scala | 38 +++++++-------- 4 files changed, 76 insertions(+), 20 deletions(-) diff --git a/src/dotty/tools/dotc/ast/tpd.scala b/src/dotty/tools/dotc/ast/tpd.scala index 9b7c9cbae810..4c21fcf49229 100644 --- a/src/dotty/tools/dotc/ast/tpd.scala +++ b/src/dotty/tools/dotc/ast/tpd.scala @@ -9,6 +9,7 @@ import SymDenotations._, Symbols._, StdNames._, Annotations._, Trees._, Symbols. import Denotations._, Decorators._ import config.Printers._ import typer.Mode +import collection.mutable import typer.ErrorReporting._ import scala.annotation.tailrec @@ -620,6 +621,12 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { } acc(false, tree) } + + def filterSubTrees(f: Tree => Boolean): List[Tree] = { + val buf = new mutable.ListBuffer[Tree] + foreachSubTree { tree => if (f(tree)) buf += tree } + buf.toList + } } implicit class ListOfTreeDecorator(val xs: List[tpd.Tree]) extends AnyVal { diff --git a/src/dotty/tools/dotc/core/Decorators.scala b/src/dotty/tools/dotc/core/Decorators.scala index 99af4d0cb42e..e0d7fae33735 100644 --- a/src/dotty/tools/dotc/core/Decorators.scala +++ b/src/dotty/tools/dotc/core/Decorators.scala @@ -100,6 +100,11 @@ object Decorators { else x1 :: xs1 } + def foldRightBN[U](z: => U)(op: (T, => U) => U): U = xs match { + case Nil => z + case x :: xs1 => op(x, xs1.foldRightBN(z)(op)) + } + final def hasSameLengthAs[U](ys: List[U]): Boolean = { @tailrec def loop(xs: List[T], ys: List[U]): Boolean = if (xs.isEmpty) ys.isEmpty diff --git a/src/dotty/tools/dotc/transform/TreeChecker.scala b/src/dotty/tools/dotc/transform/TreeChecker.scala index 1448e8bf9033..77e504e2561f 100644 --- a/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -20,6 +20,7 @@ import reporting.ThrowingReporter import ast.Trees._ import ast.{tpd, untpd} import util.SourcePosition +import collection.mutable import ProtoTypes._ import java.lang.AssertionError @@ -57,6 +58,32 @@ class TreeChecker { } class Checker(phasesToCheck: Seq[Phase]) extends ReTyper { + + val definedSyms = new mutable.HashSet[Symbol] + + def withDefinedSym[T](tree: untpd.Tree)(op: => T)(implicit ctx: Context): T = { + if (tree.isDef) { + //assert(!definedSyms.contains(tree.symbol), i"doubly defined symbol: ${tree.symbol}in $tree") + definedSyms += tree.symbol + //println(i"defined: ${tree.symbol}") + val res = op + definedSyms -= tree.symbol + //println(i"undefined: ${tree.symbol}") + res + } + else op + } + + def withDefinedSyms[T](trees: List[untpd.Tree])(op: => T)(implicit ctx: Context) = + trees.foldRightBN(op)(withDefinedSym(_)(_)) + + def withDefinedSymss[T](vparamss: List[List[untpd.ValDef]])(op: => T)(implicit ctx: Context): T = + vparamss.foldRightBN(op)(withDefinedSyms(_)(_)) + + def assertDefined(tree: untpd.Tree)(implicit ctx: Context) = + if (tree.symbol.maybeOwner.isTerm) + ()//assert(definedSyms contains tree.symbol, i"undefined symbol ${tree.symbol}") + override def typed(tree: untpd.Tree, pt: Type)(implicit ctx: Context) = { val res = tree match { case _: untpd.UnApply => @@ -88,7 +115,8 @@ class TreeChecker { override def typedIdent(tree: untpd.Ident, pt: Type)(implicit ctx: Context): Tree = { assert(tree.isTerm || !ctx.isAfterTyper, tree.show + " at " + ctx.phase) - assert(tree.isType || !needsSelect(tree.tpe), i"bad type ${tree.tpe} for $tree") + assert(tree.isType || !needsSelect(tree.tpe), i"bad type ${tree.tpe} for $tree # ${tree.uniqueId}") + assertDefined(tree) super.typedIdent(tree, pt) } @@ -117,6 +145,22 @@ class TreeChecker { super.typedClassDef(cdef, cls) } + override def typedDefDef(ddef: untpd.DefDef, sym: Symbol)(implicit ctx: Context) = + withDefinedSyms(ddef.tparams) { + withDefinedSymss(ddef.vparamss) { + super.typedDefDef(ddef, sym) + } + } + + override def typedCase(tree: untpd.CaseDef, pt: Type, selType: Type, gadtSyms: Set[Symbol])(implicit ctx: Context): CaseDef = { + withDefinedSyms(tree.pat.asInstanceOf[tpd.Tree].filterSubTrees(_.isInstanceOf[ast.Trees.Bind[_]])) { + super.typedCase(tree, pt, selType, gadtSyms) + } + } + + override def typedBlock(tree: untpd.Block, pt: Type)(implicit ctx: Context) = + withDefinedSyms(tree.stats) { super.typedBlock(tree, pt) } + /** Check that all defined symbols have legal owners. * An owner is legal if it is either the same as the context's owner * or there's an owner chain of valdefs starting at the context's owner and diff --git a/src/dotty/tools/dotc/typer/Typer.scala b/src/dotty/tools/dotc/typer/Typer.scala index 1cab2fa465ac..1afa5f9f33f9 100644 --- a/src/dotty/tools/dotc/typer/Typer.scala +++ b/src/dotty/tools/dotc/typer/Typer.scala @@ -610,29 +610,29 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit accu(Set.empty, selType) } - def typedCase(tree: untpd.CaseDef): CaseDef = track("typedCase") { - def caseRest(pat: Tree)(implicit ctx: Context) = { - gadtSyms foreach (_.resetGADTFlexType) - pat foreachSubTree { - case b: Bind => - if (ctx.scope.lookup(b.name) == NoSymbol) ctx.enter(b.symbol) - else ctx.error(d"duplicate pattern variable: ${b.name}", b.pos) - case _ => - } - val guard1 = typedExpr(tree.guard, defn.BooleanType) - val body1 = typedExpr(tree.body, pt) - assignType(cpy.CaseDef(tree)(pat, guard1, body1), body1) - } - val doCase: () => CaseDef = - () => caseRest(typedPattern(tree.pat, selType))(ctx.fresh.setNewScope) - (doCase /: gadtSyms)((op, tsym) => tsym.withGADTFlexType(op))() - } - - val cases1 = tree.cases mapconserve typedCase + val cases1 = tree.cases mapconserve (typedCase(_, pt, selType, gadtSyms)) assignType(cpy.Match(tree)(sel1, cases1), cases1) } } + def typedCase(tree: untpd.CaseDef, pt: Type, selType: Type, gadtSyms: Set[Symbol])(implicit ctx: Context): CaseDef = track("typedCase") { + def caseRest(pat: Tree)(implicit ctx: Context) = { + gadtSyms foreach (_.resetGADTFlexType) + pat foreachSubTree { + case b: Bind => + if (ctx.scope.lookup(b.name) == NoSymbol) ctx.enter(b.symbol) + else ctx.error(d"duplicate pattern variable: ${b.name}", b.pos) + case _ => + } + val guard1 = typedExpr(tree.guard, defn.BooleanType) + val body1 = typedExpr(tree.body, pt) + assignType(cpy.CaseDef(tree)(pat, guard1, body1), body1) + } + val doCase: () => CaseDef = + () => caseRest(typedPattern(tree.pat, selType))(ctx.fresh.setNewScope) + (doCase /: gadtSyms)((op, tsym) => tsym.withGADTFlexType(op))() + } + def typedReturn(tree: untpd.Return)(implicit ctx: Context): Return = track("typedReturn") { def returnProto(owner: Symbol) = if (owner.isConstructor) defn.UnitType else owner.info.finalResultType From 8b0f2d62ca3100362a70c0e175fc95d7d21a452a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 24 Oct 2014 19:38:22 +0200 Subject: [PATCH 22/33] Enabled commented out tests --- src/dotty/tools/dotc/transform/TreeChecker.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dotty/tools/dotc/transform/TreeChecker.scala b/src/dotty/tools/dotc/transform/TreeChecker.scala index 77e504e2561f..7ba570bea392 100644 --- a/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -63,7 +63,7 @@ class TreeChecker { def withDefinedSym[T](tree: untpd.Tree)(op: => T)(implicit ctx: Context): T = { if (tree.isDef) { - //assert(!definedSyms.contains(tree.symbol), i"doubly defined symbol: ${tree.symbol}in $tree") + assert(!definedSyms.contains(tree.symbol), i"doubly defined symbol: ${tree.symbol}in $tree") definedSyms += tree.symbol //println(i"defined: ${tree.symbol}") val res = op @@ -82,7 +82,7 @@ class TreeChecker { def assertDefined(tree: untpd.Tree)(implicit ctx: Context) = if (tree.symbol.maybeOwner.isTerm) - ()//assert(definedSyms contains tree.symbol, i"undefined symbol ${tree.symbol}") + assert(definedSyms contains tree.symbol, i"undefined symbol ${tree.symbol}") override def typed(tree: untpd.Tree, pt: Type)(implicit ctx: Context) = { val res = tree match { From a5878de9409c4d511c482a296ec6a3e85e868b93 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Fri, 24 Oct 2014 20:59:53 +0200 Subject: [PATCH 23/33] Fix pattern matcher double defining symbols used in type tests that are known to succeed. --- src/dotty/tools/dotc/transform/PatternMatcher.scala | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/dotty/tools/dotc/transform/PatternMatcher.scala b/src/dotty/tools/dotc/transform/PatternMatcher.scala index ac92bb80c009..4a1a66fbc458 100644 --- a/src/dotty/tools/dotc/transform/PatternMatcher.scala +++ b/src/dotty/tools/dotc/transform/PatternMatcher.scala @@ -393,12 +393,14 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans val res: Tree val nextBinder: Symbol - lazy val introducedRebindings = + lazy val introducedRebindings = /* if(nextBinder ne prevBinder) Rebindings(prevBinder, nextBinder) - else NoRebindings + else */ NoRebindings def chainBefore(next: Tree)(casegen: Casegen): Tree = - /*atPos(pos)(*/casegen.flatMapCond(cond, res, nextBinder, next)//) + if(prevBinder ne nextBinder) // happens when typeTest is known to succeed + /*atPos(pos)(*/casegen.flatMapCond(cond, res, nextBinder, next)//) + else casegen.flatMapGuard(cond, next) } // unless we're optimizing, emit local variable bindings for all subpatterns of extractor/case class patterns From f459bf085d743c801fe724089438c0082014121f Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Fri, 24 Oct 2014 21:00:51 +0200 Subject: [PATCH 24/33] Fix PreserveSubPatBinders not storing subparts that are used only for type tests. --- src/dotty/tools/dotc/transform/PatternMatcher.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/transform/PatternMatcher.scala b/src/dotty/tools/dotc/transform/PatternMatcher.scala index 4a1a66fbc458..8af1b4a214d0 100644 --- a/src/dotty/tools/dotc/transform/PatternMatcher.scala +++ b/src/dotty/tools/dotc/transform/PatternMatcher.scala @@ -428,6 +428,7 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans private lazy val (stored, substed) = (subPatBinders, subPatRefs).zipped.partition{ case (sym, _) => storedBinders(sym) } + // dd: this didn't yet trigger error. But I believe it would. if this causes double denition of symbol error this can be replaced with NoRebindings protected lazy val introducedRebindings: Rebindings = if (!emitVars) Rebindings(subPatBinders, subPatRefs) else { val (subPatBindersSubstituted, subPatRefsSubstituted) = substed.unzip @@ -948,7 +949,7 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans case Bind(nme.WILDCARD, _) => true // don't skip when binding an interesting symbol! case Ident(nme.WILDCARD) => true case Alternative(ps) => ps forall unapply - case Typed(PatternBoundToUnderscore(), _) => true + case Typed(PatternBoundToUnderscore(), _) => false // true // Dmitry: change in dotty. Type test will be performed and the field must be stored case _ => false } } From 70946d7e2f8f3ca69b2ebba63f7afe34a53946a6 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 26 Oct 2014 15:46:38 +0100 Subject: [PATCH 25/33] Better tests and bugfix for named args The original problem was that in an expression f(x = bar(y = z)) only the outer named arg was eliminated by FirstTransform. The first error was that the postcondition in FirstTransform did not get to the named arg, because it got called from the overrdden typed method in TreeChecker, yet function arguments were evaluated with typedUnadapted. action: change Retyper and TreeChecker to override typedUndapped instead of typed. This flushed out the second error: transformOther in FirstTransform needs to recursively transform the argument of a NamedArg, because the framework itself does not handle NamedArg nodes. Now, all tests pass except that TreeChecker itself fails -Ycheck:gettersSetters due to a problem with handling by-name function types. This should be fixed in a separate PR. --- src/dotty/tools/dotc/Compiler.scala | 3 ++- .../tools/dotc/transform/FirstTransform.scala | 2 +- .../tools/dotc/transform/TreeChecker.scala | 26 +++++++++++++------ src/dotty/tools/dotc/typer/ReTyper.scala | 4 +-- test/dotc/tests.scala | 2 +- 5 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/dotty/tools/dotc/Compiler.scala b/src/dotty/tools/dotc/Compiler.scala index ec899ebb8716..d141b7488f2a 100644 --- a/src/dotty/tools/dotc/Compiler.scala +++ b/src/dotty/tools/dotc/Compiler.scala @@ -53,7 +53,8 @@ class Compiler { new Literalize, new GettersSetters), List(new Erasure), - List(new CapturedVars, new Constructors), + List(new CapturedVars, + new Constructors), List(new LambdaLift) ) diff --git a/src/dotty/tools/dotc/transform/FirstTransform.scala b/src/dotty/tools/dotc/transform/FirstTransform.scala index 05255c982a93..fed47b160d9d 100644 --- a/src/dotty/tools/dotc/transform/FirstTransform.scala +++ b/src/dotty/tools/dotc/transform/FirstTransform.scala @@ -123,7 +123,7 @@ class FirstTransform extends MiniPhaseTransform with IdentityDenotTransformer { override def transformOther(tree: Tree)(implicit ctx: Context, info: TransformerInfo) = tree match { case tree: Import => EmptyTree - case tree: NamedArg => tree.arg + case tree: NamedArg => transform(tree.arg) case AppliedTypeTree(tycon, args) => val tparams = tycon.tpe.typeSymbol.typeParams Checking.checkBounds( diff --git a/src/dotty/tools/dotc/transform/TreeChecker.scala b/src/dotty/tools/dotc/transform/TreeChecker.scala index 7ba570bea392..4a7d280e5344 100644 --- a/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -54,7 +54,13 @@ class TreeChecker { val checkingCtx = ctx.fresh .setTyperState(ctx.typerState.withReporter(new ThrowingReporter(ctx.typerState.reporter))) val checker = new Checker(previousPhases(phasesToRun.toList)(ctx)) - checker.typedExpr(ctx.compilationUnit.tpdTree)(checkingCtx) + try checker.typedExpr(ctx.compilationUnit.tpdTree)(checkingCtx) + catch { + case ex: Throwable => + implicit val ctx: Context = checkingCtx + println(i"*** error while checking after phase ${checkingCtx.phase.prev} ***") + throw ex + } } class Checker(phasesToCheck: Seq[Phase]) extends ReTyper { @@ -84,17 +90,17 @@ class TreeChecker { if (tree.symbol.maybeOwner.isTerm) assert(definedSyms contains tree.symbol, i"undefined symbol ${tree.symbol}") - override def typed(tree: untpd.Tree, pt: Type)(implicit ctx: Context) = { + override def typedUnadapted(tree: untpd.Tree, pt: Type)(implicit ctx: Context): tpd.Tree = { val res = tree match { case _: untpd.UnApply => // can't recheck patterns tree.asInstanceOf[tpd.Tree] case _: untpd.TypedSplice | _: untpd.Thicket | _: EmptyValDef[_] => - super.typed(tree) + super.typedUnadapted(tree) case _ if tree.isType => promote(tree) case _ => - val tree1 = super.typed(tree, pt) + val tree1 = super.typedUnadapted(tree, pt) def isSubType(tp1: Type, tp2: Type) = (tp1 eq tp2) || // accept NoType / NoType (tp1 <:< tp2) @@ -106,9 +112,10 @@ class TreeChecker { |After checking: ${tree1.show} |Why different : """.stripMargin + core.TypeComparer.explained((tp1 <:< tp2)(_)) - assert(isSubType(tree1.tpe, tree.typeOpt), divergenceMsg(tree1.tpe, tree.typeOpt)) + if (tree.hasType) // it might not be typed because Typer sometimes constructs new untyped trees and resubmits them to typedUnadapted + assert(isSubType(tree1.tpe, tree.typeOpt), divergenceMsg(tree1.tpe, tree.typeOpt)) tree1 - } + } phasesToCheck.foreach(_.checkPostCondition(res)) res } @@ -183,10 +190,13 @@ class TreeChecker { override def adapt(tree: Tree, pt: Type, original: untpd.Tree = untpd.EmptyTree)(implicit ctx: Context) = { def isPrimaryConstructorReturn = ctx.owner.isPrimaryConstructor && pt.isRef(ctx.owner.owner) && tree.tpe.isRef(defn.UnitClass) - if (ctx.mode.isExpr && !isPrimaryConstructorReturn && !pt.isInstanceOf[FunProto]) + if (ctx.mode.isExpr && + !tree.isEmpty && + !isPrimaryConstructorReturn && + !pt.isInstanceOf[FunProto]) assert(tree.tpe <:< pt, s"error at ${sourcePos(tree.pos)}\n" + - err.typeMismatchStr(tree.tpe, pt)) + err.typeMismatchStr(tree.tpe, pt) + "tree = " + tree) tree } } diff --git a/src/dotty/tools/dotc/typer/ReTyper.scala b/src/dotty/tools/dotc/typer/ReTyper.scala index f36e8e2fd10d..713549840a0c 100644 --- a/src/dotty/tools/dotc/typer/ReTyper.scala +++ b/src/dotty/tools/dotc/typer/ReTyper.scala @@ -87,8 +87,8 @@ class ReTyper extends Typer { super.handleUnexpectedFunType(tree, fun) } - override def typed(tree: untpd.Tree, pt: Type)(implicit ctx: Context) = - try super.typed(tree, pt) + override def typedUnadapted(tree: untpd.Tree, pt: Type)(implicit ctx: Context) = + try super.typedUnadapted(tree, pt) catch { case ex: Throwable => println(i"exception while typing $tree of class ${tree.getClass} # ${tree.uniqueId}") diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index 901167a5649c..4c6d004bf216 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -106,7 +106,7 @@ class tests extends CompilerTest { @Test def dotc_core = compileDir(dotcDir + "tools/dotc/core", twice)(allowDeepSubtypes) @Test def dotc_core_pickling = compileDir(dotcDir + "tools/dotc/core/pickling", twice)(allowDeepSubtypes) - //@Test def dotc_transform = compileDir(dotcDir + "tools/dotc/transform", twice) + @Test def dotc_transform = compileDir(dotcDir + "tools/dotc/transform", twice)(defaultOptions ++ List("-Ycheck:pat,era,lam")) //disabled, awaiting fix for call-by-name function types. @Test def dotc_parsing = compileDir(dotcDir + "tools/dotc/parsing", twice) From a3ef72bb723e1eb2c14c75b958d5864ea700dabf Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 26 Oct 2014 16:52:40 +0100 Subject: [PATCH 26/33] Make LambdaLift diagnostics log messages instead of printing them directly. --- src/dotty/tools/dotc/transform/LambdaLift.scala | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index 47a61a5436a7..fc3b959c3695 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -88,7 +88,7 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this } def narrowLiftedOwner(sym: Symbol, owner: Symbol)(implicit ctx: Context) = { - println(i"narrow lifted $sym to $owner") + ctx.log(i"narrow lifted $sym to $owner") if (sym.owner.skipConstructor.isTerm && owner.isProperlyContainedIn(liftedOwner(sym))) { changedLiftedOwner = true @@ -128,7 +128,7 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this */ private def markFree(sym: Symbol, enclosure: Symbol)(implicit ctx: Context): Boolean = try { if (!enclosure.exists) throw new NoPath - println(i"mark free: ${sym.showLocated} with owner ${sym.maybeOwner} marked free in $enclosure") + ctx.log(i"mark free: ${sym.showLocated} with owner ${sym.maybeOwner} marked free in $enclosure") (enclosure == sym.enclosure) || { ctx.debuglog(i"$enclosure != ${sym.enclosure}") narrowLiftedOwner(enclosure, sym.enclosingClass) @@ -202,7 +202,6 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this private def computeFreeVars()(implicit ctx: Context): Unit = do { changedFreeVars = false - // println(s"called = ${called.toList map { case (from, to) => from.showLocated + " -> " + to.toList.map(_.showLocated) }}") for { caller <- called.keys callee <- called(caller) From 107049919d509c965dcee71fd8afd2e535058043 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 27 Oct 2014 11:11:06 +0100 Subject: [PATCH 27/33] Dropped comment. --- src/dotty/tools/dotc/transform/LambdaLift.scala | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index fc3b959c3695..196753e82d8e 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -408,16 +408,4 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this override def transformTypeDef(tree: TypeDef)(implicit ctx: Context, info: TransformerInfo) = if (needsLifting(tree.symbol)) liftDef(tree) else tree - } - - -/* done in lazyvals? - case Block(stats, expr0) => - val (lzyVals, rest) = stats partition { - case stat: ValDef => stat.symbol.isLazy || stat.symbol.isModuleVar - case _ => false - } - if (lzyVals.isEmpty) tree - else treeCopy.Block(tree, lzyVals ::: rest, expr0) - -*/ +} From 46eb5ea0b8ac3e80795e7f5030b128794feb692c Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 27 Oct 2014 17:18:48 +0100 Subject: [PATCH 28/33] Fix treatment of by name functions By-name functions like `(=> T) => T` were not treated correctly before. Witness the disabled `-Ycheck:gettersSetters` for transform/TreeCheckers in thge test suite. This commit changes the scheme how => T types are treated and fixes the problems with by-name functions. --- src/dotty/tools/dotc/TypeErasure.scala | 15 +++- .../tools/dotc/transform/ElimByName.scala | 85 ++++++++++--------- src/dotty/tools/dotc/transform/Erasure.scala | 8 +- .../tools/dotc/transform/TreeChecker.scala | 2 +- test/dotc/tests.scala | 5 +- 5 files changed, 64 insertions(+), 51 deletions(-) diff --git a/src/dotty/tools/dotc/TypeErasure.scala b/src/dotty/tools/dotc/TypeErasure.scala index 851be76583bf..2a55d6732f83 100644 --- a/src/dotty/tools/dotc/TypeErasure.scala +++ b/src/dotty/tools/dotc/TypeErasure.scala @@ -111,6 +111,12 @@ object TypeErasure { erasure(tp) } + /** The erasure of a symbol's info. This is different of `erasure` in the way `ExprType`s are + * treated. `eraseInfo` maps them them to nullary method types, whereas `erasure` maps them + * to `Function0`. + */ + def eraseInfo(tp: Type)(implicit ctx: Context): Type = scalaErasureFn.eraseInfo(tp)(erasureCtx) + /** The erasure of a function result type. Differs from normal erasure in that * Unit is kept instead of being mapped to BoxedUnit. */ @@ -135,7 +141,7 @@ object TypeErasure { if ((sym eq defn.Any_asInstanceOf) || (sym eq defn.Any_isInstanceOf)) eraseParamBounds(sym.info.asInstanceOf[PolyType]) else if (sym.isAbstractType) TypeAlias(WildcardType) else if (sym.isConstructor) outer.addParam(sym.owner.asClass, erase(tp)(erasureCtx)) - else erase(tp)(erasureCtx) + else eraseInfo(tp)(erasureCtx) } def isUnboundedGeneric(tp: Type)(implicit ctx: Context) = !( @@ -263,7 +269,7 @@ class TypeErasure(isJava: Boolean, isSemi: Boolean, isConstructor: Boolean, wild case SuperType(thistpe, supertpe) => SuperType(this(thistpe), this(supertpe)) case ExprType(rt) => - MethodType(Nil, Nil, this(rt)) + defn.FunctionClass(0).typeRef case tp: TypeProxy => this(tp.underlying) case AndType(tp1, tp2) => @@ -312,6 +318,11 @@ class TypeErasure(isJava: Boolean, isSemi: Boolean, isConstructor: Boolean, wild else JavaArrayType(this(elemtp)) } + def eraseInfo(tp: Type)(implicit ctx: Context) = tp match { + case ExprType(rt) => MethodType(Nil, Nil, erasure(rt)) + case tp => erasure(tp) + } + private def eraseDerivedValueClassRef(tref: TypeRef)(implicit ctx: Context): Type = unsupported("eraseDerivedValueClass") diff --git a/src/dotty/tools/dotc/transform/ElimByName.scala b/src/dotty/tools/dotc/transform/ElimByName.scala index 1d0307398fc3..1a37c17e4a0c 100644 --- a/src/dotty/tools/dotc/transform/ElimByName.scala +++ b/src/dotty/tools/dotc/transform/ElimByName.scala @@ -2,32 +2,52 @@ package dotty.tools.dotc package transform import TreeTransforms._ -import core.DenotTransformers._ -import core.Symbols._ -import core.SymDenotations._ -import core.Contexts._ -import core.Types._ -import core.Flags._ -import core.Decorators._ +import core._ +import DenotTransformers._ +import Symbols._ +import SymDenotations._ +import Contexts._ +import Types._ +import Flags._ +import Decorators._ import SymUtils._ +import util.Attachment import core.StdNames.nme import ast.Trees._ -/** This phase eliminates ExprTypes `=> T` and replaces them by +object ElimByName { + val ByNameArg = new Attachment.Key[Unit] +} + +/** This phase eliminates ExprTypes `=> T` as types of function parameters, and replaces them by * nullary function types. More precisely: * - * For parameter types: + * For the types of parameter symbols: * * => T ==> () => T * - * For terms: + * Note that `=> T` types are not eliminated in MnethodTypes. This is done later at erasure. + * Terms are rewritten as follows: * * x ==> x.apply() if x is a parameter that had type => T + * + * Arguments to call-by-name parameters are translated as follows. First, the argument is + * rewritten by the rules + * * e.apply() ==> e if e.apply() is an argument to a call-by-name parameter * expr ==> () => expr if other expr is an argument to a call-by-name parameter + * + * This makes the argument compatible with a parameter type of () => T, which will be the + * formal parameter type at erasure. But to be -Ycheckable until then, any argument + * ARG rewritten by the rules above is again wrapped in an application ARG.apply(), + * labelled with an `ByNameParam` attachment. + * + * Erasure will later strip wrapped `.apply()` calls with ByNameParam attachments. + * */ class ElimByName extends MiniPhaseTransform with InfoTransformer { thisTransformer => import ast.tpd._ + import ElimByName._ override def phaseName: String = "elimByName" @@ -44,9 +64,9 @@ class ElimByName extends MiniPhaseTransform with InfoTransformer { thisTransform override def transformApply(tree: Apply)(implicit ctx: Context, info: TransformerInfo): Tree = ctx.traceIndented(s"transforming ${tree.show} at phase ${ctx.phase}", show = true) { - def transformArg(arg: Tree, formal: Type): Tree = formal match { - case _: ExprType => - arg match { + def transformArg(arg: Tree, formal: Type): Tree = formal.dealias match { + case formalExpr: ExprType => + val argFun = arg match { case Apply(Select(qual, nme.apply), Nil) if qual.tpe derivesFrom defn.FunctionClass(0) => qual case _ => @@ -54,24 +74,14 @@ class ElimByName extends MiniPhaseTransform with InfoTransformer { thisTransform ctx.owner, nme.ANON_FUN, Synthetic | Method, MethodType(Nil, Nil, arg.tpe.widen)) Closure(meth, _ => arg.changeOwner(ctx.owner, meth)) } + val argApplied = argFun.select(defn.Function0_apply).appliedToNone + argApplied.putAttachment(ByNameArg, ()) + argApplied case _ => arg } - /** Given that `info` is the possibly curried) method type of the - * tree's symbol, the method type that corresponds to the current application. - */ - def matchingMethType(info: Type, tree: Tree): Type = tree match { - case Apply(fn, _) => matchingMethType(info.resultType, fn) - case _ => info - } - - val origMethType = originalDenotation(tree.fun).info match { - case pt: PolyType => pt.resultType - case mt => mt - } - - val MethodType(_, formals) = matchingMethType(origMethType, tree.fun) + val MethodType(_, formals) = tree.fun.tpe.widen val args1 = tree.args.zipWithConserve(formals)(transformArg) cpy.Apply(tree)(tree.fun, args1) } @@ -99,22 +109,13 @@ class ElimByName extends MiniPhaseTransform with InfoTransformer { thisTransform case _ => tree } - def elimByNameParams(tp: Type)(implicit ctx: Context): Type = tp match { - case tp: PolyType => - tp.derivedPolyType(tp.paramNames, tp.paramBounds, elimByNameParams(tp.resultType)) - case tp: MethodType => - tp.derivedMethodType(tp.paramNames, tp.paramTypes mapConserve transformParamInfo, - elimByNameParams(tp.resultType)) - case _ => - tp - } + override def transformValDef(tree: ValDef)(implicit ctx: Context, info: TransformerInfo): Tree = + if (exprBecomesFunction(tree.symbol)) + cpy.ValDef(tree)(tpt = tree.tpt.withType(tree.symbol.info)) + else tree - def transformParamInfo(tp: Type)(implicit ctx: Context) = tp match { - case ExprType(rt) => defn.FunctionType(Nil, rt) + def transformInfo(tp: Type, sym: Symbol)(implicit ctx: Context): Type = tp match { + case ExprType(rt) if exprBecomesFunction(sym) => defn.FunctionType(Nil, rt) case _ => tp } - - def transformInfo(tp: Type, sym: Symbol)(implicit ctx: Context): Type = - if (exprBecomesFunction(sym)) transformParamInfo(tp) - else elimByNameParams(tp) } diff --git a/src/dotty/tools/dotc/transform/Erasure.scala b/src/dotty/tools/dotc/transform/Erasure.scala index 31397e08a723..3de7ae36a57d 100644 --- a/src/dotty/tools/dotc/transform/Erasure.scala +++ b/src/dotty/tools/dotc/transform/Erasure.scala @@ -62,7 +62,7 @@ class Erasure extends Phase with DenotTransformer { thisTransformer => } } case ref => - ref.derivedSingleDenotation(ref.symbol, erasure(ref.info)) + ref.derivedSingleDenotation(ref.symbol, eraseInfo(ref.info)) } val eraser = new Erasure.Typer @@ -350,7 +350,11 @@ object Erasure extends TypeTestsCasts{ override def typedApply(tree: untpd.Apply, pt: Type)(implicit ctx: Context): Tree = { val Apply(fun, args) = tree - typedExpr(fun, FunProto(args, pt, this)) match { + if (tree.removeAttachment(ElimByName.ByNameArg).isDefined) { + val Select(qual, nme.apply) = fun + typedUnadapted(qual, pt) + } + else typedExpr(fun, FunProto(args, pt, this)) match { case fun1: Apply => // arguments passed in prototype were already passed fun1 case fun1 => diff --git a/src/dotty/tools/dotc/transform/TreeChecker.scala b/src/dotty/tools/dotc/transform/TreeChecker.scala index 4a7d280e5344..e09a83a04c4b 100644 --- a/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -196,7 +196,7 @@ class TreeChecker { !pt.isInstanceOf[FunProto]) assert(tree.tpe <:< pt, s"error at ${sourcePos(tree.pos)}\n" + - err.typeMismatchStr(tree.tpe, pt) + "tree = " + tree) + err.typeMismatchStr(tree.tpe, pt) + "\ntree = " + tree) tree } } diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index 4c6d004bf216..519fa35cefe8 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -105,10 +105,7 @@ class tests extends CompilerTest { @Test def dotc_config = compileDir(dotcDir + "tools/dotc/config", twice) @Test def dotc_core = compileDir(dotcDir + "tools/dotc/core", twice)(allowDeepSubtypes) @Test def dotc_core_pickling = compileDir(dotcDir + "tools/dotc/core/pickling", twice)(allowDeepSubtypes) - - @Test def dotc_transform = compileDir(dotcDir + "tools/dotc/transform", twice)(defaultOptions ++ List("-Ycheck:pat,era,lam")) - //disabled, awaiting fix for call-by-name function types. - + @Test def dotc_transform = compileDir(dotcDir + "tools/dotc/transform", twice) @Test def dotc_parsing = compileDir(dotcDir + "tools/dotc/parsing", twice) @Test def dotc_printing = compileDir(dotcDir + "tools/dotc/printing", twice) @Test def dotc_reporting = compileDir(dotcDir + "tools/dotc/reporting", twice) From 474b2aeb608ebbdeb639ba081f413f49459b3eff Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 28 Oct 2014 15:18:24 +0100 Subject: [PATCH 29/33] Added a test for by name functions --- src/dotty/tools/dotc/transform/ElimByName.scala | 6 ++---- tests/pos/bynamefuns.scala | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) create mode 100644 tests/pos/bynamefuns.scala diff --git a/src/dotty/tools/dotc/transform/ElimByName.scala b/src/dotty/tools/dotc/transform/ElimByName.scala index 1a37c17e4a0c..40fd48327305 100644 --- a/src/dotty/tools/dotc/transform/ElimByName.scala +++ b/src/dotty/tools/dotc/transform/ElimByName.scala @@ -40,10 +40,8 @@ object ElimByName { * This makes the argument compatible with a parameter type of () => T, which will be the * formal parameter type at erasure. But to be -Ycheckable until then, any argument * ARG rewritten by the rules above is again wrapped in an application ARG.apply(), - * labelled with an `ByNameParam` attachment. - * - * Erasure will later strip wrapped `.apply()` calls with ByNameParam attachments. - * + * labelled with a `ByNameParam` attachment. Erasure will later strip wrapped + * `.apply()` calls with ByNameParam attachments. */ class ElimByName extends MiniPhaseTransform with InfoTransformer { thisTransformer => import ast.tpd._ diff --git a/tests/pos/bynamefuns.scala b/tests/pos/bynamefuns.scala new file mode 100644 index 000000000000..5aa1df38df2c --- /dev/null +++ b/tests/pos/bynamefuns.scala @@ -0,0 +1,15 @@ +object Test { + + type LF = (=> Int) => Int + + def f(x: => Int) = x * x + + val x: LF = f + + def g = 3 + + f(11) + x(g) + x(11) + +} From bae24fd7f66cf0425dbac7afec5aa3c99c18de70 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 28 Oct 2014 15:20:48 +0100 Subject: [PATCH 30/33] Made LambdaLift capable of having minitransforms run after it. Some changes needed so that Flatten can run after LambdaLift --- .../tools/dotc/transform/LambdaLift.scala | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index 196753e82d8e..051c6065eeb8 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -278,20 +278,20 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this } } - override def init(implicit ctx: Context, info: TransformerInfo) = { - free.clear() - proxyMap.clear() - called.clear() - calledFromInner.clear() - liftedOwner.clear() - liftedDefs.clear() - assert(ctx.phase == thisTransform) - (new CollectDependencies).traverse(NoSymbol, ctx.compilationUnit.tpdTree) - computeFreeVars() - computeLiftedOwners() - generateProxies()(ctx.withPhase(thisTransform.next)) - liftLocals()(ctx.withPhase(thisTransform.next)) - } + override def init(implicit ctx: Context, info: TransformerInfo) = + ctx.atPhase(thisTransform) { implicit ctx => + free.clear() + proxyMap.clear() + called.clear() + calledFromInner.clear() + liftedOwner.clear() + liftedDefs.clear() + (new CollectDependencies).traverse(NoSymbol, ctx.compilationUnit.tpdTree) + computeFreeVars() + computeLiftedOwners() + generateProxies()(ctx.withPhase(thisTransform.next)) + liftLocals()(ctx.withPhase(thisTransform.next)) + } private def currentEnclosure(implicit ctx: Context) = ctx.owner.enclosingMethod.skipConstructor @@ -355,8 +355,12 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this } } - private def liftDef(tree: MemberDef)(implicit ctx: Context): Tree = { - liftedDefs(tree.symbol.owner) += rename(tree, tree.symbol.name) + private def liftDef(tree: MemberDef)(implicit ctx: Context, info: TransformerInfo): Tree = { + val buf = liftedDefs(tree.symbol.owner) + transformFollowing(rename(tree, tree.symbol.name)) match { + case Thicket(trees) => buf ++= trees + case tree => buf += tree + } EmptyTree } From f865d2a87b9a3b72effb3fb45ef380940c582b9e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 28 Oct 2014 15:21:33 +0100 Subject: [PATCH 31/33] New miniphase: Flatten --- src/dotty/tools/dotc/Compiler.scala | 3 +- src/dotty/tools/dotc/Flatten.scala | 15 ------ .../dotc/{ => transform}/ElimLocals.scala | 0 src/dotty/tools/dotc/transform/Flatten.scala | 49 +++++++++++++++++++ src/dotty/tools/dotc/transform/SymUtils.scala | 7 ++- .../tools/dotc/transform/TreeChecker.scala | 2 +- test/dotc/tests.scala | 2 +- 7 files changed, 59 insertions(+), 19 deletions(-) delete mode 100644 src/dotty/tools/dotc/Flatten.scala rename src/dotty/tools/dotc/{ => transform}/ElimLocals.scala (100%) create mode 100644 src/dotty/tools/dotc/transform/Flatten.scala diff --git a/src/dotty/tools/dotc/Compiler.scala b/src/dotty/tools/dotc/Compiler.scala index d141b7488f2a..cb70480ce7c9 100644 --- a/src/dotty/tools/dotc/Compiler.scala +++ b/src/dotty/tools/dotc/Compiler.scala @@ -55,7 +55,8 @@ class Compiler { List(new Erasure), List(new CapturedVars, new Constructors), - List(new LambdaLift) + List(new LambdaLift, + new Flatten) ) var runId = 1 diff --git a/src/dotty/tools/dotc/Flatten.scala b/src/dotty/tools/dotc/Flatten.scala deleted file mode 100644 index d7ccc1ae4e11..000000000000 --- a/src/dotty/tools/dotc/Flatten.scala +++ /dev/null @@ -1,15 +0,0 @@ -package dotty.tools.dotc -package transform - -import core._ -import DenotTransformers.SymTransformer -import Phases.Phase -import Contexts.Context -import SymDenotations.SymDenotation -import TreeTransforms.MiniPhaseTransform - -class Flatten extends MiniPhaseTransform with SymTransformer { thisTransformer => - override def phaseName = "flatten" - - def transformSym(ref: SymDenotation)(implicit ctx: Context) = ??? -} diff --git a/src/dotty/tools/dotc/ElimLocals.scala b/src/dotty/tools/dotc/transform/ElimLocals.scala similarity index 100% rename from src/dotty/tools/dotc/ElimLocals.scala rename to src/dotty/tools/dotc/transform/ElimLocals.scala diff --git a/src/dotty/tools/dotc/transform/Flatten.scala b/src/dotty/tools/dotc/transform/Flatten.scala new file mode 100644 index 000000000000..769503f9d6ef --- /dev/null +++ b/src/dotty/tools/dotc/transform/Flatten.scala @@ -0,0 +1,49 @@ +package dotty.tools.dotc +package transform + +import core._ +import DenotTransformers.SymTransformer +import Phases.Phase +import Contexts.Context +import Flags._ +import SymUtils._ +import SymDenotations.SymDenotation +import collection.mutable +import TreeTransforms.MiniPhaseTransform +import dotty.tools.dotc.transform.TreeTransforms.TransformerInfo + +class Flatten extends MiniPhaseTransform with SymTransformer { thisTransform => + import ast.tpd._ + override def phaseName = "flatten" + + def transformSym(ref: SymDenotation)(implicit ctx: Context) = { + if (ref.isClass && !ref.is(Package) && !ref.owner.is(Package)) { + ref.copySymDenotation( + name = ref.flatName, + owner = ref.enclosingPackageClass) + } + else ref + } + + override def treeTransformPhase = thisTransform.next + + private val liftedDefs = new mutable.ListBuffer[Tree] + + private def liftIfNested(tree: Tree)(implicit ctx: Context, info: TransformerInfo) = + if (ctx.owner is Package) tree + else { + liftedDefs += transformFollowing(tree) + EmptyTree + } + + override def transformStats(stats: List[Tree])(implicit ctx: Context, info: TransformerInfo) = + if (ctx.owner is Package) { + val liftedStats = stats ++ liftedDefs + liftedDefs.clear + liftedStats + } + else stats + + override def transformTypeDef(tree: TypeDef)(implicit ctx: Context, info: TransformerInfo) = + liftIfNested(tree) +} diff --git a/src/dotty/tools/dotc/transform/SymUtils.scala b/src/dotty/tools/dotc/transform/SymUtils.scala index 2875327c4712..ba45d3f04663 100644 --- a/src/dotty/tools/dotc/transform/SymUtils.scala +++ b/src/dotty/tools/dotc/transform/SymUtils.scala @@ -5,6 +5,7 @@ import core._ import Types._ import Contexts._ import Symbols._ +import SymDenotations._ import Decorators._ import Names._ import StdNames._ @@ -13,7 +14,8 @@ import Flags._ import language.implicitConversions object SymUtils { - implicit def decorateSymUtils(sym: Symbol): SymUtils = new SymUtils(sym) + implicit def decorateSymbol(sym: Symbol): SymUtils = new SymUtils(sym) + implicit def decorateSymDenot(d: SymDenotation): SymUtils = new SymUtils(d.symbol) } /** A decorator that provides methods on symbols @@ -64,4 +66,7 @@ class SymUtils(val self: Symbol) extends AnyVal { def field(implicit ctx: Context): Symbol = self.owner.info.decl(self.asTerm.name.fieldName).suchThat(!_.is(Method)).symbol + + /** `fullName` where `$' is the separator character */ + def flatName(implicit ctx: Context): Name = self.fullNameSeparated('$') } diff --git a/src/dotty/tools/dotc/transform/TreeChecker.scala b/src/dotty/tools/dotc/transform/TreeChecker.scala index e09a83a04c4b..f307f423728c 100644 --- a/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -178,7 +178,7 @@ class TreeChecker { override def typedStats(trees: List[untpd.Tree], exprOwner: Symbol)(implicit ctx: Context): List[Tree] = { for (tree <- trees) tree match { case tree: untpd.DefTree => checkOwner(tree) - case _: untpd.Thicket => assert(false, "unexpanded thicket in statement sequence") + case _: untpd.Thicket => assert(false, i"unexpanded thicket $tree in statement sequence $trees%\n%") case _ => } super.typedStats(trees, exprOwner) diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index 519fa35cefe8..072c3fd37ba7 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -15,7 +15,7 @@ class tests extends CompilerTest { implicit val defaultOptions = noCheckOptions ++ List( "-Yno-deep-subtypes", - "-Ycheck:patternMatcher,gettersSetters,lambdaLift" + "-Ycheck:patternMatcher,gettersSetters,flatten" ) val twice = List("#runs", "2", "-YnoDoubleBindings") From 048b484de29456ef511eb094d9bf20a46c2bcf73 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 28 Oct 2014 16:21:06 +0100 Subject: [PATCH 32/33] Generalize lift behavior between Flatten and LambdaLift To be combinable with follow-up mini-phases the lift operation needs to handle Thickets specially. This commit factors out the behavior from LambdaLift, so that Flatten can do the same thing. --- src/dotty/tools/dotc/ast/Trees.scala | 7 +++++++ src/dotty/tools/dotc/transform/Flatten.scala | 2 +- src/dotty/tools/dotc/transform/LambdaLift.scala | 5 +---- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/dotty/tools/dotc/ast/Trees.scala b/src/dotty/tools/dotc/ast/Trees.scala index f1ccfdb75e3e..f7d3d134af76 100644 --- a/src/dotty/tools/dotc/ast/Trees.scala +++ b/src/dotty/tools/dotc/ast/Trees.scala @@ -347,6 +347,11 @@ object Trees { s } + /** If this is a thicket, gerform `op` on each of its trees + * otherwise, perform `op` ion tree itself. + */ + def foreachInThicket(op: Tree[T] => Unit): Unit = op(this) + override def toText(printer: Printer) = printer.toText(this) override def hashCode(): Int = System.identityHashCode(this) @@ -809,6 +814,8 @@ object Trees { val newTrees = trees.map(_.withPos(pos)) new Thicket[T](newTrees).asInstanceOf[this.type] } + override def foreachInThicket(op: Tree[T] => Unit): Unit = + trees foreach (_.foreachInThicket(op)) } class EmptyValDef[T >: Untyped] extends ValDef[T]( diff --git a/src/dotty/tools/dotc/transform/Flatten.scala b/src/dotty/tools/dotc/transform/Flatten.scala index 769503f9d6ef..dceefc0bc90b 100644 --- a/src/dotty/tools/dotc/transform/Flatten.scala +++ b/src/dotty/tools/dotc/transform/Flatten.scala @@ -32,7 +32,7 @@ class Flatten extends MiniPhaseTransform with SymTransformer { thisTransform => private def liftIfNested(tree: Tree)(implicit ctx: Context, info: TransformerInfo) = if (ctx.owner is Package) tree else { - liftedDefs += transformFollowing(tree) + transformFollowing(tree).foreachInThicket(liftedDefs += _) EmptyTree } diff --git a/src/dotty/tools/dotc/transform/LambdaLift.scala b/src/dotty/tools/dotc/transform/LambdaLift.scala index 051c6065eeb8..f36ff6247a3a 100644 --- a/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -357,10 +357,7 @@ class LambdaLift extends MiniPhaseTransform with IdentityDenotTransformer { this private def liftDef(tree: MemberDef)(implicit ctx: Context, info: TransformerInfo): Tree = { val buf = liftedDefs(tree.symbol.owner) - transformFollowing(rename(tree, tree.symbol.name)) match { - case Thicket(trees) => buf ++= trees - case tree => buf += tree - } + transformFollowing(rename(tree, tree.symbol.name)).foreachInThicket(buf += _) EmptyTree } From e6eb6804661de159a9f5cf21b9347b9b93534161 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 29 Oct 2014 11:52:43 +0100 Subject: [PATCH 33/33] New phase: RestoreScopes Cleans up after LambdaLift and Flatten. RestoreScopes exhibited a problem (double definition) when compiling Unpickler. The root of the problem was in Applications.scala. The effect was that arguments woulkd be lifted out, but then the argument expression would be used anyway. That caused a closure to be present twice which caused the double def error much later. -Ycheck did not catch it because the two closure expressions were in non-overlapping scopes. --- src/dotty/tools/dotc/Compiler.scala | 3 +- .../tools/dotc/transform/RestoreScopes.scala | 36 +++++++++++++++++++ src/dotty/tools/dotc/typer/Applications.scala | 2 +- test/dotc/tests.scala | 2 +- 4 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 src/dotty/tools/dotc/transform/RestoreScopes.scala diff --git a/src/dotty/tools/dotc/Compiler.scala b/src/dotty/tools/dotc/Compiler.scala index cb70480ce7c9..d4fa7e671104 100644 --- a/src/dotty/tools/dotc/Compiler.scala +++ b/src/dotty/tools/dotc/Compiler.scala @@ -56,7 +56,8 @@ class Compiler { List(new CapturedVars, new Constructors), List(new LambdaLift, - new Flatten) + new Flatten, + new RestoreScopes) ) var runId = 1 diff --git a/src/dotty/tools/dotc/transform/RestoreScopes.scala b/src/dotty/tools/dotc/transform/RestoreScopes.scala new file mode 100644 index 000000000000..4a42523266ec --- /dev/null +++ b/src/dotty/tools/dotc/transform/RestoreScopes.scala @@ -0,0 +1,36 @@ +package dotty.tools.dotc +package transform + +import core._ +import DenotTransformers.IdentityDenotTransformer +import Contexts.Context +import Symbols._ +import Scopes._ +import collection.mutable +import TreeTransforms.MiniPhaseTransform +import ast.Trees._ +import TreeTransforms.TransformerInfo + +/** The preceding lambda lift and flatten phases move symbols to different scopes + * and rename them. This miniphase cleans up afterwards and makes sure that all + * class scopes contain the symbols defined in them. + */ +class RestoreScopes extends MiniPhaseTransform with IdentityDenotTransformer { thisTransform => + import ast.tpd._ + override def phaseName = "restoreScopes" + + override def treeTransformPhase = thisTransform.next + + override def transformTypeDef(tree: TypeDef)(implicit ctx: Context, info: TransformerInfo) = { + val TypeDef(_, _, Template(constr, _, _, body)) = tree + val restoredDecls = newScope + for (stat <- constr :: body) + if (stat.isInstanceOf[MemberDef] && stat.symbol.exists) + restoredDecls.enter(stat.symbol) + val cinfo = tree.symbol.asClass.classInfo + tree.symbol.copySymDenotation( + info = cinfo.derivedClassInfo( // Dotty deviation: Cannot expand cinfo inline without a type error + decls = restoredDecls: Scope)).installAfter(thisTransform) + tree + } +} diff --git a/src/dotty/tools/dotc/typer/Applications.scala b/src/dotty/tools/dotc/typer/Applications.scala index 035f19028d7c..a237e7781822 100644 --- a/src/dotty/tools/dotc/typer/Applications.scala +++ b/src/dotty/tools/dotc/typer/Applications.scala @@ -461,7 +461,7 @@ trait Applications extends Compatibility { self: Typer => val result = { var typedArgs = typedArgBuf.toList - val app0 = cpy.Apply(app)(normalizedFun, typedArgs) + def app0 = cpy.Apply(app)(normalizedFun, typedArgs) // needs to be a `def` because typedArgs can change later val app1 = if (!success) app0.withType(ErrorType) else { diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index 072c3fd37ba7..0d30d806d95c 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -15,7 +15,7 @@ class tests extends CompilerTest { implicit val defaultOptions = noCheckOptions ++ List( "-Yno-deep-subtypes", - "-Ycheck:patternMatcher,gettersSetters,flatten" + "-Ycheck:patternMatcher,gettersSetters,restoreScopes" ) val twice = List("#runs", "2", "-YnoDoubleBindings")