From 9b6cd3d77f87439787f325d1bd87a50719140412 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 22 Jun 2020 09:06:28 +0200 Subject: [PATCH 1/3] Fix #9213: handle valdefs in mixin parent constructors --- .../dotty/tools/dotc/transform/Mixin.scala | 60 ++++++++++++++++++- tests/pos/i9213.scala | 8 +++ 2 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 tests/pos/i9213.scala diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala index ad1a1ea77455..64ce080c97f4 100644 --- a/compiler/src/dotty/tools/dotc/transform/Mixin.scala +++ b/compiler/src/dotty/tools/dotc/transform/Mixin.scala @@ -1,4 +1,5 @@ -package dotty.tools.dotc +package dotty.tools +package dotc package transform import core._ @@ -194,7 +195,24 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => for (p <- impl.parents; constr = stripBlock(p).symbol if constr.isConstructor) yield constr.owner -> transformConstructor(p) ).toMap - val superCalls = superCallsAndArgs.transform((_, v) => v._1) + + /** Definitions in a parent trait constructor call (these can arise + * due to reorderings with named and/or default parameters). + */ + def prefix(tree: Tree): List[Tree] = + if stripBlock(tree).symbol.owner.is(Trait) then + tree match + case Block(stats, expr) => stats ::: prefix(expr) + case _ => Nil + else Nil + + /** the proper trait parent constructor call, without any preceding val defs */ + def properCall(tree: Tree): Tree = + val call = stripBlock(tree) + if call.symbol.owner.is(Trait) then call else tree + + val prefixes = superCallsAndArgs.transform((_, v) => prefix(v._1)) + val superCalls = superCallsAndArgs.transform((_, v) => properCall(v._1)) val initArgs = superCallsAndArgs.transform((_, v) => v._2) def superCallOpt(baseCls: Symbol): List[Tree] = superCalls.get(baseCls) match { @@ -211,6 +229,38 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => def wasOneOf(sym: Symbol, flags: FlagSet) = ctx.atPhase(thisPhase) { sym.isOneOf(flags) } + /** The prefix definitions of a mixin parent constructor, lifted + * to the enclosing class. + */ + def traitConstrPrefix(mixin: ClassSymbol): List[Tree] = + prefixes.get(mixin) match + case Some(stats) => + stats.map { + case stat: ValDef => + stat.symbol.copySymDenotation( + owner = cls, + initFlags = stat.symbol.flags | PrivateLocal + ).installAfter(thisPhase) + stat.symbol.enteredAfter(thisPhase) + stat + } + case _ => + Nil + + /** Adapt tree so that references to valdefs that are lifted to the + * class now use `this` as a prefix + */ + def adaptToPrefix(stat: Tree, prefixSyms: List[Symbol]) = + if prefixSyms.isEmpty then stat + else + val m = new TreeMap: + override def transform(tree: Tree)(using Context) = tree match + case tree: Ident if prefixSyms.contains(tree.symbol) => + This(cls).select(tree.symbol).withSpan(tree.span) + case _ => + super.transform(tree) + m.transform(stat) + def traitInits(mixin: ClassSymbol): List[Tree] = { var argNum = 0 def nextArgument() = initArgs.get(mixin) match { @@ -275,7 +325,11 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => if (cls.is(Trait)) traitDefs(impl.body) else if (!cls.isPrimitiveValueClass) { val mixInits = mixins.flatMap { mixin => - flatten(traitInits(mixin)) ::: superCallOpt(mixin) ::: setters(mixin) ::: mixinForwarders(mixin) + val prefix = traitConstrPrefix(mixin) + val prefixSyms = prefix.map(_.symbol) + val initsAndCall = (flatten(traitInits(mixin)) ::: superCallOpt(mixin)) + .map(adaptToPrefix(_, prefixSyms)) + prefix ::: initsAndCall ::: setters(mixin) ::: mixinForwarders(mixin) } superCallOpt(superCls) ::: mixInits ::: impl.body } diff --git a/tests/pos/i9213.scala b/tests/pos/i9213.scala new file mode 100644 index 000000000000..39c6a70d62f2 --- /dev/null +++ b/tests/pos/i9213.scala @@ -0,0 +1,8 @@ +trait A(a: Any, b: Int) { + //var x = 0 +} +// class A(a: String, b: Int) { +// +// } //OK! + object B extends A(b = 0, a = String("")) +// object B extends A(a = String(""), b = 0) //OK! \ No newline at end of file From f4520d4f6e593a5b172f196612cb3ed30479a988 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 22 Jun 2020 11:40:45 +0200 Subject: [PATCH 2/3] Simplify implementation --- .../dotty/tools/dotc/transform/Mixin.scala | 121 +++++++----------- 1 file changed, 44 insertions(+), 77 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala index 64ce080c97f4..62f898b81692 100644 --- a/compiler/src/dotty/tools/dotc/transform/Mixin.scala +++ b/compiler/src/dotty/tools/dotc/transform/Mixin.scala @@ -177,98 +177,61 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => }) ++ initBuf } - /** Map constructor call to a pair of a supercall and a list of arguments - * to be used as initializers of trait parameters if the target of the call - * is a trait. + /** Map constructor call to a triple of a supercall, and if the target + * is a trait + * - a list of val defs used in arguments (these can arise + * due to reorderings with named and/or default parameters). + * - a list of arguments to be used as initializers of trait parameters */ - def transformConstructor(tree: Tree): (Tree, List[Tree]) = tree match { + def transformConstructor(tree: Tree): (Tree, List[Tree], List[Tree]) = tree match { case Block(stats, expr) => - val (scall, inits) = transformConstructor(expr) - (cpy.Block(tree)(stats, scall), inits) + val (scall, inits, args) = transformConstructor(expr) + if args.isEmpty then + (cpy.Block(tree)(stats, scall), inits, args) + else // it's a trait constructor with parameters, lift all prefix statements to class context + // so that they precede argument definitions. + stats.foreach { + case stat: ValDef => + stat.symbol.copySymDenotation( + owner = cls, + initFlags = stat.symbol.flags | PrivateLocal + ).installAfter(thisPhase) + stat.symbol.enteredAfter(thisPhase) + } + (scall, stats ::: inits, args) case _ => val Apply(sel @ Select(New(_), nme.CONSTRUCTOR), args) = tree val (callArgs, initArgs) = if (tree.symbol.owner.is(Trait)) (Nil, args) else (args, Nil) - (superRef(tree.symbol, tree.span).appliedToArgs(callArgs), initArgs) + (superRef(tree.symbol, tree.span).appliedToArgs(callArgs), Nil, initArgs) } - val superCallsAndArgs = ( + val superCallsAndArgs: Map[Symbol, (Tree, List[Tree], List[Tree])] = ( for (p <- impl.parents; constr = stripBlock(p).symbol if constr.isConstructor) yield constr.owner -> transformConstructor(p) ).toMap - /** Definitions in a parent trait constructor call (these can arise - * due to reorderings with named and/or default parameters). - */ - def prefix(tree: Tree): List[Tree] = - if stripBlock(tree).symbol.owner.is(Trait) then - tree match - case Block(stats, expr) => stats ::: prefix(expr) - case _ => Nil - else Nil - - /** the proper trait parent constructor call, without any preceding val defs */ - def properCall(tree: Tree): Tree = - val call = stripBlock(tree) - if call.symbol.owner.is(Trait) then call else tree - - val prefixes = superCallsAndArgs.transform((_, v) => prefix(v._1)) - val superCalls = superCallsAndArgs.transform((_, v) => properCall(v._1)) - val initArgs = superCallsAndArgs.transform((_, v) => v._2) - - def superCallOpt(baseCls: Symbol): List[Tree] = superCalls.get(baseCls) match { - case Some(call) => + def superCallOpt(baseCls: Symbol): List[Tree] = + superCallsAndArgs.get(baseCls) match + case Some((call, _, _)) => if (defn.NotRuntimeClasses.contains(baseCls) || baseCls.isAllOf(NoInitsTrait)) Nil else call :: Nil case None => - if (baseCls.isAllOf(NoInitsTrait) || defn.NoInitClasses.contains(baseCls) || defn.isFunctionClass(baseCls)) Nil + if baseCls.isAllOf(NoInitsTrait) || defn.NoInitClasses.contains(baseCls) || defn.isFunctionClass(baseCls) then + Nil else //println(i"synth super call ${baseCls.primaryConstructor}: ${baseCls.primaryConstructor.info}") transformFollowingDeep(superRef(baseCls.primaryConstructor).appliedToNone) :: Nil - } def wasOneOf(sym: Symbol, flags: FlagSet) = ctx.atPhase(thisPhase) { sym.isOneOf(flags) } - /** The prefix definitions of a mixin parent constructor, lifted - * to the enclosing class. - */ - def traitConstrPrefix(mixin: ClassSymbol): List[Tree] = - prefixes.get(mixin) match - case Some(stats) => - stats.map { - case stat: ValDef => - stat.symbol.copySymDenotation( - owner = cls, - initFlags = stat.symbol.flags | PrivateLocal - ).installAfter(thisPhase) - stat.symbol.enteredAfter(thisPhase) - stat - } - case _ => - Nil - - /** Adapt tree so that references to valdefs that are lifted to the - * class now use `this` as a prefix - */ - def adaptToPrefix(stat: Tree, prefixSyms: List[Symbol]) = - if prefixSyms.isEmpty then stat - else - val m = new TreeMap: - override def transform(tree: Tree)(using Context) = tree match - case tree: Ident if prefixSyms.contains(tree.symbol) => - This(cls).select(tree.symbol).withSpan(tree.span) - case _ => - super.transform(tree) - m.transform(stat) - def traitInits(mixin: ClassSymbol): List[Tree] = { - var argNum = 0 - def nextArgument() = initArgs.get(mixin) match { - case Some(arguments) => - val result = arguments(argNum) - argNum += 1 - result - case None => + val argsIt = superCallsAndArgs.get(mixin) match + case Some((_, _, args)) => args.iterator + case _ => Iterator.empty + def nextArgument() = + if argsIt.hasNext then argsIt.next + else assert( impl.parents.forall(_.tpe.typeSymbol != mixin), i"missing parameters for $mixin from $impl should have been caught in typer") @@ -277,7 +240,6 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => |needs to be implemented directly so that arguments can be passed""", cls.sourcePos) EmptyTree - } for (getter <- mixin.info.decls.toList if getter.isGetter && !wasOneOf(getter, Deferred)) yield { val isScala2x = mixin.is(Scala2x) @@ -325,13 +287,18 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => if (cls.is(Trait)) traitDefs(impl.body) else if (!cls.isPrimitiveValueClass) { val mixInits = mixins.flatMap { mixin => - val prefix = traitConstrPrefix(mixin) - val prefixSyms = prefix.map(_.symbol) - val initsAndCall = (flatten(traitInits(mixin)) ::: superCallOpt(mixin)) - .map(adaptToPrefix(_, prefixSyms)) - prefix ::: initsAndCall ::: setters(mixin) ::: mixinForwarders(mixin) + val prefix = superCallsAndArgs.get(mixin) match + case Some((_, inits, _)) => inits + case _ => Nil + prefix + ::: flatten(traitInits(mixin)) + ::: superCallOpt(mixin) + ::: setters(mixin) + ::: mixinForwarders(mixin) } - superCallOpt(superCls) ::: mixInits ::: impl.body + superCallOpt(superCls) + ::: mixInits + ::: impl.body } else impl.body) } From b99df13088bfa1561648d0a22a75082cbc562dda Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 22 Jun 2020 11:44:58 +0200 Subject: [PATCH 3/3] Add to test case --- .../src/dotty/tools/dotc/transform/Mixin.scala | 3 +-- tests/pos/i9213.scala | 17 +++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala index 62f898b81692..055e415b9919 100644 --- a/compiler/src/dotty/tools/dotc/transform/Mixin.scala +++ b/compiler/src/dotty/tools/dotc/transform/Mixin.scala @@ -210,8 +210,7 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase => yield constr.owner -> transformConstructor(p) ).toMap - def superCallOpt(baseCls: Symbol): List[Tree] = - superCallsAndArgs.get(baseCls) match + def superCallOpt(baseCls: Symbol): List[Tree] = superCallsAndArgs.get(baseCls) match case Some((call, _, _)) => if (defn.NotRuntimeClasses.contains(baseCls) || baseCls.isAllOf(NoInitsTrait)) Nil else call :: Nil diff --git a/tests/pos/i9213.scala b/tests/pos/i9213.scala index 39c6a70d62f2..1a6488c4c0fb 100644 --- a/tests/pos/i9213.scala +++ b/tests/pos/i9213.scala @@ -1,8 +1,9 @@ -trait A(a: Any, b: Int) { - //var x = 0 -} -// class A(a: String, b: Int) { -// -// } //OK! - object B extends A(b = 0, a = String("")) -// object B extends A(a = String(""), b = 0) //OK! \ No newline at end of file +trait A(a: Any, b: Int) +trait B(a: Any, b: Int): + var x = 0 +class C(a: String, b: Int) + +object O extends + C(b = 0, a = String("")), + A(b = 0, a = String("")), + B(b = 0, a = String(""))