From 3b14fcab2196b947a39ed5ae664b4764f5cab1b7 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 13 Apr 2022 10:09:48 +0200 Subject: [PATCH 1/5] Also hoist lifted arguments in super calls Super calls with default and named parameters can have lifted arguments in val defs preceding the constructor call. If these are complex, we need to hoist them out as well. Fixes #14164 --- .../tools/dotc/transform/HoistSuperArgs.scala | 14 ++++++++++++-- tests/run/i14164.scala | 13 +++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 tests/run/i14164.scala diff --git a/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala b/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala index 3233601310ae..d12c5ce6739d 100644 --- a/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala +++ b/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala @@ -173,13 +173,23 @@ class HoistSuperArgs extends MiniPhase with IdentityDenotTransformer { thisPhase } } + /** Hoist super arg from a lifted parameter that gets evaluated before the call */ + def hoistSuperArgFromDef(paramDef: Tree, cdef: DefDef): Tree = paramDef match + case vdef: ValDef => + cpy.ValDef(vdef)(rhs = hoistSuperArg(vdef.rhs, cdef)) + case ddef: DefDef => + cpy.DefDef(ddef)(rhs = hoistSuperArg(ddef.rhs, cdef)) + case _ => + paramDef + /** Hoist complex arguments in super call out of the class. */ - def hoistSuperArgsFromCall(superCall: Tree, cdef: DefDef): Tree = superCall match { + def hoistSuperArgsFromCall(superCall: Tree, cdef: DefDef): Tree = superCall match + case Block(defs, expr) => + cpy.Block(superCall)(defs.mapconserve(hoistSuperArgFromDef(_, cdef)), hoistSuperArgsFromCall(expr, cdef)) case Apply(fn, args) => cpy.Apply(superCall)(hoistSuperArgsFromCall(fn, cdef), args.mapconserve(hoistSuperArg(_, cdef))) case _ => superCall - } /** Hoist complex arguments in this-constructor call of secondary constructor out of the class. */ def hoistSuperArgsFromConstr(stat: Tree): Tree = stat match { diff --git a/tests/run/i14164.scala b/tests/run/i14164.scala new file mode 100644 index 000000000000..fb340e303cd6 --- /dev/null +++ b/tests/run/i14164.scala @@ -0,0 +1,13 @@ +object Test: + class Base(a: String = "x", param: String) + + class Child extends Base( + param = + for x <- Seq("a") yield x + "param" + ) + + new Child + + def main(args: Array[String]) = () + From 02654397c735490276d648868dd3330f08206bd8 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 13 Apr 2022 14:59:41 +0200 Subject: [PATCH 2/5] Take lifted arguments into accountfor supercall argumemts Since we never housted arguments if the supercall had lifted parameters, we did not exercise that case before. Some fixes were necessary to make it work. --- .../tools/dotc/transform/HoistSuperArgs.scala | 68 +++++++++++-------- tests/run/i14164.scala | 14 ++++ 2 files changed, 54 insertions(+), 28 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala b/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala index d12c5ce6739d..565e39e40369 100644 --- a/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala +++ b/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala @@ -60,11 +60,12 @@ class HoistSuperArgs extends MiniPhase with IdentityDenotTransformer { thisPhase /** If argument is complex, hoist it out into its own method and refer to the * method instead. - * @param arg The argument that might be hoisted - * @param cdef The definition of the constructor from which the call is made + * @param arg The argument that might be hoisted + * @param cdef The definition of the constructor from which the call is made + * @param lifted Argument definitions that were lifted out in a call prefix * @return The argument after possible hoisting */ - private def hoistSuperArg(arg: Tree, cdef: DefDef): Tree = { + private def hoistSuperArg(arg: Tree, cdef: DefDef, lifted: List[Symbol]): Tree = { val constr = cdef.symbol lazy val origParams = // The parameters that can be accessed in the supercall if (constr == cls.primaryConstructor) @@ -92,11 +93,14 @@ class HoistSuperArgs extends MiniPhase with IdentityDenotTransformer { thisPhase val argTypeWrtConstr = argType.widenTermRefExpr.subst(origParams, allParamRefs(constr.info)) // argType with references to paramRefs of the primary constructor instead of // local parameter accessors + val abstractedArgType = + if lifted.isEmpty then argTypeWrtConstr + else MethodType.fromSymbols(lifted, argTypeWrtConstr) newSymbol( owner = methOwner, name = SuperArgName.fresh(cls.name.toTermName), flags = Synthetic | Private | Method | staticFlag, - info = replaceResult(constr.info, argTypeWrtConstr), + info = replaceResult(constr.info, abstractedArgType), coord = constr.coord ).enteredAfter(thisPhase) } @@ -122,6 +126,7 @@ class HoistSuperArgs extends MiniPhase with IdentityDenotTransformer { thisPhase case ntp: NamedType => val owner = ntp.symbol.maybeOwner (owner == cls || owner == constr) && ntp.symbol.isParamOrAccessor + || lifted.contains(ntp.symbol) case _ => false } @@ -134,7 +139,7 @@ class HoistSuperArgs extends MiniPhase with IdentityDenotTransformer { thisPhase if pref.isType then pref.tpe.typeSymbol else pref.symbol) val tmap = new TreeTypeMap( typeMap = new TypeMap { - lazy val origToParam = origParams.zip(paramSyms).toMap + lazy val origToParam = (origParams ::: lifted).zip(paramSyms).toMap def apply(tp: Type) = tp match { case tp: NamedType if needsRewire(tp) => origToParam.get(tp.symbol) match { @@ -164,45 +169,52 @@ class HoistSuperArgs extends MiniPhase with IdentityDenotTransformer { thisPhase Nil } val (typeParams, termParams) = origParams.span(_.isType) - val res = ref(superMeth) + var res = ref(superMeth) .appliedToTypes(typeParams.map(_.typeRef)) .appliedToArgss(termParamRefs(constr.info, termParams)) + if lifted.nonEmpty then + res = res.appliedToArgs(lifted.map(ref)) report.log(i"hoist $arg, cls = $cls = $res") res case _ => arg } } - /** Hoist super arg from a lifted parameter that gets evaluated before the call */ - def hoistSuperArgFromDef(paramDef: Tree, cdef: DefDef): Tree = paramDef match - case vdef: ValDef => - cpy.ValDef(vdef)(rhs = hoistSuperArg(vdef.rhs, cdef)) - case ddef: DefDef => - cpy.DefDef(ddef)(rhs = hoistSuperArg(ddef.rhs, cdef)) - case _ => - paramDef - /** Hoist complex arguments in super call out of the class. */ - def hoistSuperArgsFromCall(superCall: Tree, cdef: DefDef): Tree = superCall match + def hoistSuperArgsFromCall(superCall: Tree, cdef: DefDef, lifted: mutable.ListBuffer[Symbol]): Tree = superCall match case Block(defs, expr) => - cpy.Block(superCall)(defs.mapconserve(hoistSuperArgFromDef(_, cdef)), hoistSuperArgsFromCall(expr, cdef)) + cpy.Block(superCall)( + stats = defs.mapconserve { + case vdef: ValDef => + try cpy.ValDef(vdef)(rhs = hoistSuperArg(vdef.rhs, cdef, lifted.toList)) + finally lifted += vdef.symbol + case ddef: DefDef => + try cpy.DefDef(ddef)(rhs = hoistSuperArg(ddef.rhs, cdef, lifted.toList)) + finally lifted += ddef.symbol + case stat => + stat + }, + expr = hoistSuperArgsFromCall(expr, cdef, lifted)) case Apply(fn, args) => - cpy.Apply(superCall)(hoistSuperArgsFromCall(fn, cdef), args.mapconserve(hoistSuperArg(_, cdef))) + cpy.Apply(superCall)( + hoistSuperArgsFromCall(fn, cdef, lifted), + args.mapconserve(hoistSuperArg(_, cdef, lifted.toList))) case _ => superCall /** Hoist complex arguments in this-constructor call of secondary constructor out of the class. */ def hoistSuperArgsFromConstr(stat: Tree): Tree = stat match { - case stat: DefDef if stat.symbol.isClassConstructor => - cpy.DefDef(stat)(rhs = - stat.rhs match { - case Block(superCall :: stats, expr) => - val superCall1 = hoistSuperArgsFromCall(superCall, stat) - if (superCall1 eq superCall) stat.rhs - else cpy.Block(stat.rhs)(superCall1 :: stats, expr) + case constr: DefDef if constr.symbol.isClassConstructor => + val lifted = new mutable.ListBuffer[Symbol] + cpy.DefDef(constr)(rhs = + constr.rhs match + case Block(stats @ (superCall :: stats1), expr: Literal) => + cpy.Block(constr.rhs)( + stats.derivedCons(hoistSuperArgsFromCall(superCall, constr, lifted), stats1), + expr) case _ => - hoistSuperArgsFromCall(stat.rhs, stat) - }) + hoistSuperArgsFromCall(constr.rhs, constr, lifted) + ) case _ => stat } @@ -212,7 +224,7 @@ class HoistSuperArgs extends MiniPhase with IdentityDenotTransformer { thisPhase tdef.rhs match { case impl @ Template(cdef, superCall :: others, _, _) => val hoist = new Hoister(tdef.symbol) - val hoistedSuperCall = hoist.hoistSuperArgsFromCall(superCall, cdef) + val hoistedSuperCall = hoist.hoistSuperArgsFromCall(superCall, cdef, new mutable.ListBuffer) val hoistedBody = impl.body.mapconserve(hoist.hoistSuperArgsFromConstr) if (hoist.superArgDefs.isEmpty) tdef else { diff --git a/tests/run/i14164.scala b/tests/run/i14164.scala index fb340e303cd6..2a8476625634 100644 --- a/tests/run/i14164.scala +++ b/tests/run/i14164.scala @@ -1,3 +1,4 @@ + object Test: class Base(a: String = "x", param: String) @@ -11,3 +12,16 @@ object Test: def main(args: Array[String]) = () +end Test + +class Test2: + class Inner(withDefault: String = "inner")( + dependentDefault: String = withDefault) extends Object { + def this(x: Int) = this(x.toString)() + } + +class Test3: + class Inner(withDefault: () => String = () => "inner")( + dependentDefault: String = withDefault()) extends Object { + def this(x: Int) = this(() => x.toString)() + } From dc52f844bb01079b8c3512e377dfa19754f40752 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 13 Apr 2022 15:45:37 +0200 Subject: [PATCH 3/5] Don't always hoist super arguments referring to outer classes --- compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala b/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala index 565e39e40369..edbfbd1552c4 100644 --- a/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala +++ b/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala @@ -107,19 +107,18 @@ class HoistSuperArgs extends MiniPhase with IdentityDenotTransformer { thisPhase /** Type of a reference implies that it needs to be hoisted */ def refNeedsHoist(tp: Type): Boolean = tp match { - case tp: ThisType => !tp.cls.isStaticOwner && tp.cls != cls + case tp: ThisType => !tp.cls.isStaticOwner && !cls.isContainedIn(tp.cls) case tp: TermRef => refNeedsHoist(tp.prefix) case _ => false } /** Super call argument is complex, needs to be hoisted */ - def needsHoist(tree: Tree) = tree match { + def needsHoist(tree: Tree) = tree match case _: DefDef => true case _: Template => true case _: New => !tree.tpe.typeSymbol.isStatic case _: RefTree | _: This => refNeedsHoist(tree.tpe) case _ => false - } /** Only rewire types that are owned by the current Hoister and is an param or accessor */ def needsRewire(tp: Type) = tp match { From 19ae93ba82f4bf4758bab3c2a9f9721898be7c1b Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 14 Apr 2022 19:42:48 +0200 Subject: [PATCH 4/5] Drop serializabilty tests in parallel collections CB --- community-build/community-projects/scala-parallel-collections | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/community-build/community-projects/scala-parallel-collections b/community-build/community-projects/scala-parallel-collections index b26e74437f0c..c43c0a2523f6 160000 --- a/community-build/community-projects/scala-parallel-collections +++ b/community-build/community-projects/scala-parallel-collections @@ -1 +1 @@ -Subproject commit b26e74437f0cadc3c92e9378e69197f3494b1811 +Subproject commit c43c0a2523f62551f476f1b901168c6c876f1e4c From 077652ce304e0ceedc5a07b0d0ca6c92ba5f60a0 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 14 Apr 2022 22:03:19 +0200 Subject: [PATCH 5/5] Fix correct test file in parallel collections CB --- community-build/community-projects/scala-parallel-collections | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/community-build/community-projects/scala-parallel-collections b/community-build/community-projects/scala-parallel-collections index c43c0a2523f6..a6bd648bb188 160000 --- a/community-build/community-projects/scala-parallel-collections +++ b/community-build/community-projects/scala-parallel-collections @@ -1 +1 @@ -Subproject commit c43c0a2523f62551f476f1b901168c6c876f1e4c +Subproject commit a6bd648bb188a65ab36be07e956e52fe25f64d67