From e5cad0c2ccfd4e8ea3dfb9f0eb22ab0a958287f8 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 11 Mar 2016 10:47:27 +0100 Subject: [PATCH 1/5] Refinements to referencesOuter In a New we need to decide based on the prefix of the type of object created. --- .../tools/dotc/transform/ExplicitOuter.scala | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/src/dotty/tools/dotc/transform/ExplicitOuter.scala b/src/dotty/tools/dotc/transform/ExplicitOuter.scala index 4cf076c45d3f..9170cd277c22 100644 --- a/src/dotty/tools/dotc/transform/ExplicitOuter.scala +++ b/src/dotty/tools/dotc/transform/ExplicitOuter.scala @@ -212,27 +212,34 @@ object ExplicitOuter { /** Tree references an outer class of `cls` which is not a static owner. */ def referencesOuter(cls: Symbol, tree: Tree)(implicit ctx: Context): Boolean = { - def isOuter(sym: Symbol) = + def isOuterSym(sym: Symbol) = !sym.isStaticOwner && cls.isProperlyContainedIn(sym) + def isOuterRef(ref: Type): Boolean = ref match { + case ref: ThisType => + isOuterSym(ref.cls) + case ref: TermRef => + if (ref.prefix ne NoPrefix) + !ref.symbol.isStatic && isOuterRef(ref.prefix) + else if (ref.symbol is Hoistable) + // ref.symbol will be placed in enclosing class scope by LambdaLift, so it might need + // an outer path then. + isOuterSym(ref.symbol.owner.enclosingClass) + else + // ref.symbol will get a proxy in immediately enclosing class. If this properly + // contains the current class, it needs an outer path. + ctx.owner.enclosingClass.owner.enclosingClass.isContainedIn(ref.symbol.owner) + case _ => false + } + def hasOuterPrefix(tp: Type) = tp match { + case TypeRef(prefix, _) => isOuterRef(prefix) + case _ => false + } tree match { - case thisTree @ This(_) => - isOuter(thisTree.symbol) - case id: Ident => - id.tpe match { - case ref @ TermRef(NoPrefix, _) => - if (ref.symbol is Hoistable) - // ref.symbol will be placed in enclosing class scope by LambdaLift, so it might need - // an outer path then. - isOuter(ref.symbol.owner.enclosingClass) - else - // ref.symbol will get a proxy in immediately enclosing class. If this properly - // contains the current class, it needs an outer path. - ctx.owner.enclosingClass.owner.enclosingClass.isContainedIn(ref.symbol.owner) - case _ => false - } + case _: This | _: Ident => isOuterRef(tree.tpe) case nw: New => val newCls = nw.tpe.classSymbol - isOuter(newCls.owner.enclosingClass) || + isOuterSym(newCls.owner.enclosingClass) || + hasOuterPrefix(nw.tpe) || newCls.owner.isTerm && cls.isProperlyContainedIn(newCls) // newCls might get proxies for free variables. If current class is // properly contained in newCls, it needs an outer path to newCls access the From d34a9e3d89a8e205e23a6d59e02ccf736dd572f4 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 11 Mar 2016 10:48:33 +0100 Subject: [PATCH 2/5] Refinement to TreeTypeMap When recursing in a template body, need to update the context's owner, so that `ref` can work correctly. --- src/dotty/tools/dotc/ast/TreeTypeMap.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/ast/TreeTypeMap.scala b/src/dotty/tools/dotc/ast/TreeTypeMap.scala index d714a3d21f40..856a471ddd03 100644 --- a/src/dotty/tools/dotc/ast/TreeTypeMap.scala +++ b/src/dotty/tools/dotc/ast/TreeTypeMap.scala @@ -82,9 +82,11 @@ final class TreeTypeMap( constr = tmap.transformSub(constr), parents = parents mapconserve transform, self = tmap.transformSub(self), - body = impl.body mapconserve tmap.transform + body = impl.body mapconserve + (tmap.transform(_)(ctx.withOwner(mapOwner(impl.symbol.owner)))) ).withType(tmap.mapType(impl.tpe)) case tree1 => + println(i"tree type map $tree1, owner = ${ctx.owner}") tree1.withType(mapType(tree1.tpe)) match { case id: Ident if tpd.needsSelect(id.tpe) => ref(id.tpe.asInstanceOf[TermRef]).withPos(id.pos) From 69b07e2466ae96c37066ee02c7d45b9b52417342 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 11 Mar 2016 10:48:59 +0100 Subject: [PATCH 3/5] Test case The test fails if either of the previous two commits is missing. --- tests/pos/i1131.scala | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 tests/pos/i1131.scala diff --git a/tests/pos/i1131.scala b/tests/pos/i1131.scala new file mode 100644 index 000000000000..42c2a685c9b0 --- /dev/null +++ b/tests/pos/i1131.scala @@ -0,0 +1,14 @@ +class Outer { + class Inner +} + +trait MustBeATrait { + val o = new Outer + val inner = { + class C { + new o.Inner + } + new C + } + val inner2 = new o.Inner {} +} From dad9dfc2cc83e58fbe59f8cffd28ec1180d6fab1 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 11 Mar 2016 16:13:31 +0100 Subject: [PATCH 4/5] Changes to owners in Mixin and Constructors Two changes: 1. Replace changeOwer with changeOwnerAfter for code that moves into the $initial methods in Mixin. This is needed because otherwise subsequent transforms gets confused wrt new vs old owners. `i1131.scala` exhibits the problem. 2. Drop `transformSym` changed the owner of tenplate-local symbols to be the primary constructor. But that is done anyway with a "changeOwnerAfter" in `intoConstr`. So it is redundant and actually gets in the way with a `changeOwnerAfter` in `Mixin`. The faulty scenario is this: 1. The SymTransformer of Constructor is run on a constructor-local definition. The owner of that definition is set to after phase Constructors. 2. The body of the definition is transformed in Mixin. The owner is set to the initializer method, but only for the interval between Mixin and Constructors. Changing to changeOwner in Mixin avoided that problem by duplicating the symbol but it runs into other problems. Fortunately, the solution is much simpler than the status quo: Two changeOwnerAfter calls and no SymTransformer. --- src/dotty/tools/dotc/transform/Constructors.scala | 14 +------------- src/dotty/tools/dotc/transform/Mixin.scala | 4 ++-- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/dotty/tools/dotc/transform/Constructors.scala b/src/dotty/tools/dotc/transform/Constructors.scala index b6ebd7d90393..44638ce4892e 100644 --- a/src/dotty/tools/dotc/transform/Constructors.scala +++ b/src/dotty/tools/dotc/transform/Constructors.scala @@ -26,7 +26,7 @@ import collection.mutable * - also moves private fields that are accessed only from constructor * into the constructor if possible. */ -class Constructors extends MiniPhaseTransform with SymTransformer { thisTransform => +class Constructors extends MiniPhaseTransform with IdentityDenotTransformer { thisTransform => import tpd._ override def phaseName: String = "constructors" @@ -99,18 +99,6 @@ class Constructors extends MiniPhaseTransform with SymTransformer { thisTransfor } } - /** Symbols that are owned by either or a class field move into the - * primary constructor. - */ - override def transformSym(sym: SymDenotation)(implicit ctx: Context): SymDenotation = { - def ownerBecomesConstructor(owner: Symbol): Boolean = - (owner.isLocalDummy || owner.isTerm && !owner.is(MethodOrLazy)) && - owner.owner.isClass - if (ownerBecomesConstructor(sym.owner)) - sym.copySymDenotation(owner = sym.owner.enclosingClass.primaryConstructor) - else sym - } - /** @return true if after ExplicitOuter, all references from this tree go via an * outer link, so no parameter accessors need to be rewired to parameters */ diff --git a/src/dotty/tools/dotc/transform/Mixin.scala b/src/dotty/tools/dotc/transform/Mixin.scala index 32b268fc75f9..d9e0c74769b0 100644 --- a/src/dotty/tools/dotc/transform/Mixin.scala +++ b/src/dotty/tools/dotc/transform/Mixin.scala @@ -134,8 +134,8 @@ class Mixin extends MiniPhaseTransform with SymTransformer { thisTransform => val vsym = stat.symbol val isym = initializer(vsym) val rhs = Block( - initBuf.toList.map(_.changeOwner(impl.symbol, isym)), - stat.rhs.changeOwner(vsym, isym).wildcardToDefault) + initBuf.toList.map(_.changeOwnerAfter(impl.symbol, isym, thisTransform)), + stat.rhs.changeOwnerAfter(vsym, isym, thisTransform).wildcardToDefault) initBuf.clear() cpy.DefDef(stat)(rhs = EmptyTree) :: DefDef(isym, rhs) :: Nil case stat: DefDef if stat.symbol.isSetter => From 5f2c21b738f2de1c73f5625d7811da86e26eb6bc Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 12 Mar 2016 14:32:02 +0100 Subject: [PATCH 5/5] Drop debug println --- src/dotty/tools/dotc/ast/TreeTypeMap.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/src/dotty/tools/dotc/ast/TreeTypeMap.scala b/src/dotty/tools/dotc/ast/TreeTypeMap.scala index 856a471ddd03..a35fe2e8ffe8 100644 --- a/src/dotty/tools/dotc/ast/TreeTypeMap.scala +++ b/src/dotty/tools/dotc/ast/TreeTypeMap.scala @@ -86,7 +86,6 @@ final class TreeTypeMap( (tmap.transform(_)(ctx.withOwner(mapOwner(impl.symbol.owner)))) ).withType(tmap.mapType(impl.tpe)) case tree1 => - println(i"tree type map $tree1, owner = ${ctx.owner}") tree1.withType(mapType(tree1.tpe)) match { case id: Ident if tpd.needsSelect(id.tpe) => ref(id.tpe.asInstanceOf[TermRef]).withPos(id.pos)