From e0ada72e87b93d44fed73cdfdea89bae32bac7fa Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 15 Nov 2020 13:00:28 +0100 Subject: [PATCH 1/2] Fix #9886: Special handling for by-name parameters of traits --- .../dotty/tools/dotc/transform/ElimByName.scala | 16 +++++++++++++--- .../src/dotty/tools/dotc/transform/Getters.scala | 10 ++++++++++ tests/pos/i9886.scala | 11 +++++++++++ 3 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 tests/pos/i9886.scala diff --git a/compiler/src/dotty/tools/dotc/transform/ElimByName.scala b/compiler/src/dotty/tools/dotc/transform/ElimByName.scala index aeed5ac76d39..e43e673a4e4a 100644 --- a/compiler/src/dotty/tools/dotc/transform/ElimByName.scala +++ b/compiler/src/dotty/tools/dotc/transform/ElimByName.scala @@ -8,6 +8,8 @@ import Contexts._ import Types._ import core.StdNames.nme import ast.Trees._ +import Decorators._ +import Flags._ /** This phase eliminates ExprTypes `=> T` as types of method parameter references, and replaces them b * nullary function types. More precisely: @@ -62,13 +64,21 @@ class ElimByName extends TransformByNameApply with InfoTransformer { override def transformValDef(tree: ValDef)(using Context): Tree = atPhase(next) { - if (exprBecomesFunction(tree.symbol)) - cpy.ValDef(tree)(tpt = tree.tpt.withType(tree.symbol.info)) + val sym = tree.symbol + if (exprBecomesFunction(sym)) + if sym.is(ParamAccessor) && sym.owner.is(Trait) then + cpy.DefDef(tree)(tree.name, Nil, Nil, tree.tpt.withType(sym.info.widenExpr), tree.rhs) + else + cpy.ValDef(tree)(tpt = tree.tpt.withType(sym.info)) else tree } def transformInfo(tp: Type, sym: Symbol)(using Context): Type = tp match { - case ExprType(rt) => defn.FunctionOf(Nil, rt) + case ExprType(rt) => + if sym.is(ParamAccessor) && sym.owner.is(Trait) then + ExprType(defn.FunctionOf(Nil, rt)) + else + defn.FunctionOf(Nil, rt) case _ => tp } diff --git a/compiler/src/dotty/tools/dotc/transform/Getters.scala b/compiler/src/dotty/tools/dotc/transform/Getters.scala index 7e5ea9c81b2a..8f8bca8095cf 100644 --- a/compiler/src/dotty/tools/dotc/transform/Getters.scala +++ b/compiler/src/dotty/tools/dotc/transform/Getters.scala @@ -12,6 +12,7 @@ import Flags._ import ValueClasses._ import SymUtils._ import NameOps._ +import Decorators._ import collection.mutable @@ -81,6 +82,15 @@ class Getters extends MiniPhase with SymTransformer { thisPhase => // seem to be a problem since references to a getter don't care whether // it's a `T` or a `=> T` } + else if d.info.isInstanceOf[ExprType] + && d.is(ParamAccessor) && d.owner.is(Trait) + && !d.is(Method) + then + // Value parameters of traits get an accessor by the first clause of the + // definition of `d1` above, but by-name parameters don't since they are not + // value types. I.e. we do not want to replace `=> T` with `=> (=> T)`. + // But we still need to mark them as accessors. + d.copySymDenotation(initFlags = d.flags | AccessorCreationFlags) else d // Drop the Local flag from all private[this] and protected[this] members. diff --git a/tests/pos/i9886.scala b/tests/pos/i9886.scala new file mode 100644 index 000000000000..8e92f322d125 --- /dev/null +++ b/tests/pos/i9886.scala @@ -0,0 +1,11 @@ +class A(l: Any): + def f = l + +class B(l: => Any): + def f = l + +trait C(l: Any): + def f = l + +trait D(l: => Any): + def f = l From 89ed2afc70f2529acbf7f4de0c2b6938d1a5788f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 15 Nov 2020 17:53:15 +0100 Subject: [PATCH 2/2] WIP --- compiler/src/dotty/tools/dotc/Compiler.scala | 2 +- .../tools/dotc/transform/ElimByName.scala | 1 + .../dotty/tools/dotc/transform/Memoize.scala | 11 +++++-- .../dotty/tools/dotc/transform/Mixin.scala | 26 ++++++++++------ .../dotty/tools/dotc/transform/MixinOps.scala | 4 +-- tests/run/i9886.scala | 31 +++++++++++++++++++ 6 files changed, 59 insertions(+), 16 deletions(-) create mode 100644 tests/run/i9886.scala diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index 5e0e711febe0..3b4e160d68ba 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -102,7 +102,7 @@ class Compiler { new ElimPolyFunction, // Rewrite PolyFunction subclasses to FunctionN subclasses new TailRec, // Rewrite tail recursion to loops new CompleteJavaEnums, // Fill in constructors for Java enums - new Mixin, // Expand trait fields and trait initializers + new Mixin) :: List( // Expand trait fields and trait initializers new LazyVals, // Expand lazy vals new Memoize, // Add private fields to getters and setters new NonLocalReturns, // Expand non-local returns diff --git a/compiler/src/dotty/tools/dotc/transform/ElimByName.scala b/compiler/src/dotty/tools/dotc/transform/ElimByName.scala index e43e673a4e4a..397884d62664 100644 --- a/compiler/src/dotty/tools/dotc/transform/ElimByName.scala +++ b/compiler/src/dotty/tools/dotc/transform/ElimByName.scala @@ -67,6 +67,7 @@ class ElimByName extends TransformByNameApply with InfoTransformer { val sym = tree.symbol if (exprBecomesFunction(sym)) if sym.is(ParamAccessor) && sym.owner.is(Trait) then + println("TVD") cpy.DefDef(tree)(tree.name, Nil, Nil, tree.tpt.withType(sym.info.widenExpr), tree.rhs) else cpy.ValDef(tree)(tpt = tree.tpt.withType(sym.info)) diff --git a/compiler/src/dotty/tools/dotc/transform/Memoize.scala b/compiler/src/dotty/tools/dotc/transform/Memoize.scala index f46c3d14fbf9..896a70024358 100644 --- a/compiler/src/dotty/tools/dotc/transform/Memoize.scala +++ b/compiler/src/dotty/tools/dotc/transform/Memoize.scala @@ -12,7 +12,7 @@ import SymUtils._ import Constants._ import ast.Trees._ import MegaPhase._ -import NameKinds.TraitSetterName +import NameKinds.{TraitSetterName, ExpandedName} import NameOps._ import Flags._ import Decorators._ @@ -116,8 +116,12 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase => } if sym.is(Accessor, butNot = NoFieldNeeded) then - def adaptToField(field: Symbol, tree: Tree): Tree = - if (tree.isEmpty) tree else tree.ensureConforms(field.info.widen) + def adaptToField(field: Symbol, rhs: Tree): Tree = + if (rhs.isEmpty) rhs + else + if (sym.name.is(ExpandedName)) + println(i"adapt to field $tree, ${sym.nextOverriddenSymbol.showLocated}") + rhs.ensureConforms(field.info.widen) def isErasableBottomField(field: Symbol, cls: Symbol): Boolean = !field.isVolatile && ((cls eq defn.NothingClass) || (cls eq defn.NullClass) || (cls eq defn.BoxedUnitClass)) @@ -137,6 +141,7 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase => if isErasableBottomField(field, rhsClass) then erasedBottomTree(rhsClass) else transformFollowingDeep(ref(field))(using ctx.withOwner(sym)) val getterDef = cpy.DefDef(tree)(rhs = getterRhs) +// println(i"add getter $getterDef, $sym, ${sym.flagsString}, ${rhs.info}") addAnnotations(fieldDef.denot) removeUnwantedAnnotations(sym, defn.GetterMetaAnnot) Thicket(fieldDef, getterDef) diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala index d9bcb5fcf544..80abaf572b34 100644 --- a/compiler/src/dotty/tools/dotc/transform/Mixin.scala +++ b/compiler/src/dotty/tools/dotc/transform/Mixin.scala @@ -253,17 +253,23 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => for (getter <- mixin.info.decls.toList if getter.isGetter && !wasOneOf(getter, Deferred)) yield { if (isCurrent(getter) || getter.name.is(ExpandedName)) { - val rhs = - if (wasOneOf(getter, ParamAccessor)) - nextArgument() - else if (getter.is(Lazy, butNot = Module)) - transformFollowing(superRef(getter).appliedToNone) - else if (getter.is(Module)) - New(getter.info.resultType, List(This(cls))) + val (rhs, toDrop) = + if wasOneOf(getter, ParamAccessor) then + val arg = nextArgument() + if arg.tpe.classSymbol == defn.FunctionClass(0) + && getter.initial.info.isInstanceOf[ExprType] + then + (arg.select(defn.Function0_apply).appliedToNone, Accessor) + else + (arg, EmptyFlags) + else if getter.is(Lazy, butNot = Module) then + (transformFollowing(superRef(getter).appliedToNone), EmptyFlags) + else if getter.is(Module) then + (New(getter.info.resultType, List(This(cls))), EmptyFlags) else - Underscore(getter.info.resultType) + (Underscore(getter.info.resultType), EmptyFlags) // transformFollowing call is needed to make memoize & lazy vals run - transformFollowing(DefDef(mkForwarderSym(getter.asTerm), rhs)) + transformFollowing(DefDef(mkForwarderSym(getter.asTerm, dropFlags = toDrop), rhs)) } else EmptyTree } @@ -280,7 +286,7 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => for (meth <- mixin.info.decls.toList if needsMixinForwarder(meth)) yield { util.Stats.record("mixin forwarders") - transformFollowing(polyDefDef(mkForwarderSym(meth.asTerm, Bridge), forwarderRhsFn(meth))) + transformFollowing(polyDefDef(mkForwarderSym(meth.asTerm, extraFlags = Bridge), forwarderRhsFn(meth))) } cpy.Template(impl)( diff --git a/compiler/src/dotty/tools/dotc/transform/MixinOps.scala b/compiler/src/dotty/tools/dotc/transform/MixinOps.scala index a563f6eabe03..c015828bfea0 100644 --- a/compiler/src/dotty/tools/dotc/transform/MixinOps.scala +++ b/compiler/src/dotty/tools/dotc/transform/MixinOps.scala @@ -18,11 +18,11 @@ class MixinOps(cls: ClassSymbol, thisPhase: DenotTransformer)(using Context) { map(n => getClassIfDefined("org.junit." + n)). filter(_.exists) - def mkForwarderSym(member: TermSymbol, extraFlags: FlagSet = EmptyFlags): TermSymbol = { + def mkForwarderSym(member: TermSymbol, extraFlags: FlagSet = EmptyFlags, dropFlags: FlagSet = EmptyFlags): TermSymbol = { val res = member.copy( owner = cls, name = member.name.stripScala2LocalSuffix, - flags = member.flags &~ Deferred &~ Module | Synthetic | extraFlags, + flags = member.flags &~ Deferred &~ Module &~ dropFlags | Synthetic | extraFlags, info = cls.thisType.memberInfo(member)).enteredAfter(thisPhase).asTerm res.addAnnotations(member.annotations.filter(_.symbol != defn.TailrecAnnot)) res diff --git a/tests/run/i9886.scala b/tests/run/i9886.scala new file mode 100644 index 000000000000..227f54235f27 --- /dev/null +++ b/tests/run/i9886.scala @@ -0,0 +1,31 @@ +class A(l: Any): + def f = l + +class B(l: => Any): + def f = l + +trait C(l: Any): + def f = l + +trait D(l: => Int): + def f = l + +object Test extends App: + var x = 0 + //object c extends C(x) + object d extends D({ x += 1; println("called"); x }) + assert(d.f + d.f == 3, x) + + +/* + +trait D(l: => Int): + def f = l + +@main def Test = + var x = 0 + val d = new D( { x += 1; x }){} + assert(d.f + d.f == 3)*/ + + +