From 6c7f2b6460de1aa6d15a6005921ca50e98a54027 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Thu, 5 Jul 2012 11:12:46 +0200 Subject: SI-5907, SI-5009 case-class copy defaults only for first param list `copy` no longer returns anonymous functions if there are multiple parameter lists, reverts most of 40e7cab7a2. Cleaned up the special type completer for copy methods. --- .../scala/tools/nsc/typechecker/Namers.scala | 43 ++++---- .../scala/tools/nsc/typechecker/Unapplies.scala | 73 ++++++++----- test/files/run/t5009.check | 1 + test/files/run/t5009.scala | 7 +- test/files/run/t5907.check | 31 ++++++ test/files/run/t5907.scala | 118 +++++++++++++++++++++ 6 files changed, 218 insertions(+), 55 deletions(-) create mode 100644 test/files/run/t5907.check create mode 100644 test/files/run/t5907.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index decd18b599..6428173577 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -502,32 +502,29 @@ trait Namers extends MethodSynthesis { noDuplicates(selectors map (_.rename), AppearsTwice) } - def enterCopyMethodOrGetter(tree: Tree, tparams: List[TypeDef]): Symbol = { - val sym = tree.symbol - val lazyType = completerOf(tree, tparams) - def completeCopyFirst = sym.isSynthetic && (!sym.hasDefault || sym.owner.info.member(nme.copy).isSynthetic) - def completeCopyMethod(clazz: Symbol) { - // the 'copy' method of case classes needs a special type completer to make - // bug0054.scala (and others) work. the copy method has to take exactly the same - // parameter types as the primary constructor. + def enterCopyMethod(copyDefDef: Tree, tparams: List[TypeDef]): Symbol = { + val sym = copyDefDef.symbol + val lazyType = completerOf(copyDefDef, tparams) + + /** Assign the types of the class parameters to the parameters of the + * copy method. See comment in `Unapplies.caseClassCopyMeth` */ + def assignParamTypes() { + val clazz = sym.owner val constructorType = clazz.primaryConstructor.tpe - val subst = new SubstSymMap(clazz.typeParams, tparams map (_.symbol)) - val vparamss = tree match { case x: DefDef => x.vparamss ; case _ => Nil } - val cparamss = constructorType.paramss - - map2(vparamss, cparamss)((vparams, cparams) => - map2(vparams, cparams)((param, cparam) => - // need to clone the type cparam.tpe??? - // problem is: we don't have the new owner yet (the new param symbol) - param.tpt setType subst(cparam.tpe) + val subst = new SubstSymMap(clazz.typeParams, tparams map (_.symbol)) + val classParamss = constructorType.paramss + val DefDef(_, _, _, copyParamss, _, _) = copyDefDef + + map2(copyParamss, classParamss)((copyParams, classParams) => + map2(copyParams, classParams)((copyP, classP) => + copyP.tpt setType subst(classP.tpe) ) ) } - sym setInfo { - mkTypeCompleter(tree) { copySym => - if (completeCopyFirst) - completeCopyMethod(copySym.owner) + sym setInfo { + mkTypeCompleter(copyDefDef) { sym => + assignParamTypes() lazyType complete sym } } @@ -604,8 +601,8 @@ trait Namers extends MethodSynthesis { val bridgeFlag = if (mods hasAnnotationNamed tpnme.bridgeAnnot) BRIDGE else 0 val sym = assignAndEnterSymbol(tree) setFlag bridgeFlag - if (name == nme.copy || tree.symbol.name.startsWith(nme.copy + nme.DEFAULT_GETTER_STRING)) - enterCopyMethodOrGetter(tree, tparams) + if (name == nme.copy && sym.isSynthetic) + enterCopyMethod(tree, tparams) else sym setInfo completerOf(tree, tparams) } diff --git a/src/compiler/scala/tools/nsc/typechecker/Unapplies.scala b/src/compiler/scala/tools/nsc/typechecker/Unapplies.scala index 4c20d14406..1b89f3db44 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Unapplies.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Unapplies.scala @@ -197,42 +197,61 @@ trait Unapplies extends ast.TreeDSL ) } + /** + * Generates copy methods for case classes. Copy only has defaults on the first + * parameter list, as of SI-5009. + * + * The parameter types of the copy method need to be exactly the same as the parameter + * types of the primary constructor. Just copying the TypeTree is not enough: a type `C` + * might refer to something else *inside* the class (i.e. as parameter type of `copy`) + * than *outside* the class (i.e. in the class parameter list). + * + * One such example is t0054.scala: + * class A { + * case class B(x: C) extends A { def copy(x: C = x) = ... } + * class C {} ^ ^ + * } (1) (2) + * + * The reference (1) to C is `A.this.C`. The reference (2) is `B.this.C` - not the same. + * + * This is fixed with a hack currently. `Unapplies.caseClassCopyMeth`, which creates the + * copy method, uses empty `TypeTree()` nodes for parameter types. + * + * In `Namers.enterDefDef`, the copy method gets a special type completer (`enterCopyMethod`). + * Before computing the body type of `copy`, the class parameter types are assigned the copy + * method parameters. + * + * This attachment class stores the copy method parameter ValDefs as an attachment in the + * ClassDef of the case class. + */ def caseClassCopyMeth(cdef: ClassDef): Option[DefDef] = { def isDisallowed(vd: ValDef) = isRepeatedParamType(vd.tpt) || isByNameParamType(vd.tpt) - val cparamss = constrParamss(cdef) - val flat = cparamss.flatten + val classParamss = constrParamss(cdef) - if (cdef.symbol.hasAbstractFlag || (flat exists isDisallowed)) None + if (cdef.symbol.hasAbstractFlag || mexists(classParamss)(isDisallowed)) None else { + def makeCopyParam(vd: ValDef, putDefault: Boolean) = { + val rhs = if (putDefault) toIdent(vd) else EmptyTree + val flags = PARAM | (vd.mods.flags & IMPLICIT) | (if (putDefault) DEFAULTPARAM else 0) + // empty tpt: see comment above + val tpt = atPos(vd.pos.focus)(TypeTree() setOriginal vd.tpt) + treeCopy.ValDef(vd, Modifiers(flags), vd.name, tpt, rhs) + } + val tparams = cdef.tparams map copyUntypedInvariant - // the parameter types have to be exactly the same as the constructor's parameter types; so it's - // not good enough to just duplicated the (untyped) tpt tree; the parameter types are removed here - // and re-added in ``finishWith'' in the namer. - def paramWithDefault(vd: ValDef) = - treeCopy.ValDef(vd, vd.mods | DEFAULTPARAM, vd.name, atPos(vd.pos.focus)(TypeTree() setOriginal vd.tpt), toIdent(vd)) - - val (copyParamss, funParamss) = cparamss match { - case Nil => (Nil, Nil) + val paramss = classParamss match { + case Nil => Nil case ps :: pss => - (List(ps.map(paramWithDefault)), mmap(pss)(p => copyUntyped[ValDef](p).copy(rhs = EmptyTree))) + ps.map(makeCopyParam(_, putDefault = true)) :: mmap(pss)(makeCopyParam(_, putDefault = false)) } val classTpe = classType(cdef, tparams) - val bodyTpe = funParamss.foldRight(classTpe)((params, restp) => gen.scalaFunctionConstr(params.map(_.tpt), restp)) - - val argss = copyParamss match { - case Nil => Nil - case ps :: _ => mmap(ps :: funParamss)(toIdent) - } - def mkFunction(vparams: List[ValDef], body: Tree) = Function(vparams, body) - val body = funParamss.foldRight(New(classTpe, argss): Tree)(mkFunction) - // [Eugene++] no longer compiles after I moved the `Function` case class into scala.reflect.internal - // val body = funParamss.foldRight(New(classTpe, argss): Tree)(Function) - - Some(atPos(cdef.pos.focus)( - DefDef(Modifiers(SYNTHETIC), nme.copy, tparams, copyParamss, bodyTpe, - body) - )) + val argss = mmap(paramss)(toIdent) + val body: Tree = New(classTpe, argss) + val copyDefDef = atPos(cdef.pos.focus)( + DefDef(Modifiers(SYNTHETIC), nme.copy, tparams, paramss, TypeTree(), body) + ) + Some(copyDefDef) } } } diff --git a/test/files/run/t5009.check b/test/files/run/t5009.check index cc9df54b34..6c567227b5 100644 --- a/test/files/run/t5009.check +++ b/test/files/run/t5009.check @@ -1,4 +1,5 @@ C(1,true) 10 C(7283,20) +C(66,-3) 100 diff --git a/test/files/run/t5009.scala b/test/files/run/t5009.scala index b4fe1bc894..db12c0d685 100644 --- a/test/files/run/t5009.scala +++ b/test/files/run/t5009.scala @@ -6,12 +6,9 @@ object Test extends App { println(c) println(c.l) - val f1a = c.copy(y = 20, x = 7283) + println(c.copy(y = 20, x = 7283)("enwa", b = false)(l = -1, s = new Object)) - val f1b = c.copy[Int, String, Object](y = 20, x = 7283) - val f2b = f1b("lkdjen", false) - val res = f2b(new Object, 100) + val res = c.copy[Int, String, Object](y = -3, x = 66)("lkdjen", false)(new Object, 100) println(res) println(res.l) - } diff --git a/test/files/run/t5907.check b/test/files/run/t5907.check new file mode 100644 index 0000000000..bc23692679 --- /dev/null +++ b/test/files/run/t5907.check @@ -0,0 +1,31 @@ +c1: 2 +c1: 2873 +c2: 37 +c3: 1, 2, 27 +c3: 1, 22, 27 +c3: 11, 7, 27 +c4: 1 +c4: 23 +c5: 1, 2, 33, b +c5: 1, 19, 33, b +c5: 1, 2, 193, c +c5: 1, 371, 193, c +c5: -1, 2, -2, lken +c6: 29, 18, -12 +c6: 1, 93, 2892 +c6: 1, 93, 761 +c7: 1, 22, 33, elkj +c7: 1, 283, 29872, me +c7: 37, 298, 899, ekjr +c8: 172, 989, 77, eliurna +c8: 1, 82, 2111, schtring +c8: -1, 92, 29, lken +c9: 1, 271, ehebab +c9: 1, 299, enag +c9: 1, 299, enag +c9: 1, 299, enag +c9: -42, 99, flae +c9: 10, 298, 27 +c9: elkn, en, emn +c9: ka, kb, kb +c9: ka, kb, ka diff --git a/test/files/run/t5907.scala b/test/files/run/t5907.scala new file mode 100644 index 0000000000..a005e9fbd3 --- /dev/null +++ b/test/files/run/t5907.scala @@ -0,0 +1,118 @@ +object Test extends App { + t + + def t { + val c1 = C1()(1) + println(c1.copy()(2)) + + { + implicit val i = 2873 + println(c1.copy()) + } + + val c2 = C2()(1) + println(c2.copy()(37)) + + val c3 = C3(1,2)(3) + println(c3.copy()(27)) + println(c3.copy(y = 22)(27)) + println(c3.copy(y = 7, x = 11)(27)) + + val c4 = C4(1) + println(c4.copy()) + println(c4.copy(x = 23)) + + val c5 = C5(1,2)(3,"a") + println(c5.copy()(33,"b")) + println(c5.copy(y = 19)(33,"b")) + + { + implicit val i = 193 + implicit val s = "c" + println(c5.copy()) + println(c5.copy(y = 371)) + println(c5.copy(x = -1)(-2, "lken")) + } + + val c6 = C6(1)(2)(3) + println(c6.copy(29)(18)(-12)) + + { + implicit val i = 2892 + println(c6.copy(x = 1)(93)) + println(c6.copy(x = 1)(93)(761)) + } + + val c7 = C7(1)(2)(3)("h") + println(c7.copy()(22)(33)("elkj")) + + { + implicit val s = "me" + println(c7.copy()(283)(29872)) + println(c7.copy(37)(298)(899)("ekjr")) + } + + val c8 = C8(1)(2,3)()("els") + println(c8.copy(x = 172)(989, 77)()("eliurna")) + + { + implicit val s = "schtring" + println(c8.copy()(82,2111)()) + println(c8.copy(x = -1)(92,29)()("lken")) + } + + val c9 = C9(1)(2)()()("u") + println(c9.copy()(271)()()("ehebab")) + + { + implicit val s = "enag" + println(c9.copy()(299)) + println(c9.copy()(299)()) + println(c9.copy()(299)()()) + println(c9.copy(x = -42)(99)()()("flae")) + } + + class KA { override def toString = "ka" } + class KB extends KA { override def toString = "kb" } + val c10 = C10(10)(3)(19) + println(c10.copy()(298)(27)) + println(c10.copy("elkn")("en")("emn")) + println(c10.copy(new KA)(new KB)(new KB)) + + { + implicit val k = new KA + println(c10.copy(new KA)(new KB)) + } + } +} + +case class C1(implicit x: Int) { + override def toString = s"c1: $x" +} +case class C2()(y: Int) { + override def toString = s"c2: $y" +} +case class C3(x: Int, y: Int)(z: Int) { + override def toString = s"c3: $x, $y, $z" +} +case class C4(x: Int) { + override def toString = s"c4: $x" +} +case class C5(x: Int, y: Int)(implicit z: Int, s: String) { + override def toString = s"c5: $x, $y, $z, $s" +} +case class C6(x: Int)(y: Int)(implicit z: Int) { + override def toString = s"c6: $x, $y, $z" +} +case class C7(x: Int)(y: Int)(z: Int)(implicit s: String) { + override def toString = s"c7: $x, $y, $z, $s" +} +case class C8(x: Int)(y: Int, z: Int)()(implicit s: String) { + override def toString = s"c8: $x, $y, $z, $s" +} +case class C9(x: Int)(y: Int)()()(implicit s: String) { + override def toString = s"c9: $x, $y, $s" +} +case class C10[T,U <: T](x: T)(y: U)(implicit z: T) { + override def toString = s"c9: $x, $y, $z" +} -- cgit v1.2.3