From f327b3e321a244919e9684c5a18e62e5e6d61728 Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Thu, 27 Apr 2017 07:25:31 +0200 Subject: [PATCH 1/5] Remove ProductN parent on case classes This means the compiler needs to systematically synthesise productElement and productArity, which aligns with what scalac is doing. This change gets rid of last reference to scala.ProductN! :tada: --- .../src/dotty/tools/dotc/ast/Desugar.scala | 30 ++++++------------- .../dotty/tools/dotc/core/Definitions.scala | 2 -- tests/neg/t1843-variances.scala | 2 +- 3 files changed, 10 insertions(+), 24 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 013516cd5c89..63491b408dc6 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -291,7 +291,8 @@ object desugar { case _ => false } - val isCaseClass = mods.is(Case) && !mods.is(Module) + val isCaseClass = mods.is(Case) && !mods.is(Module) + val isCaseObject = mods.is(Case) && mods.is(Module) val isEnum = mods.hasMod[Mod.Enum] val isEnumCase = isLegalEnumCase(cdef) val isValueClass = parents.nonEmpty && isAnyVal(parents.head) @@ -360,7 +361,7 @@ object desugar { // pN: TN = pN: @uncheckedVariance)(moreParams) = // new C[...](p1, ..., pN)(moreParams) // - // Above arity 22 we also synthesize: + // To add to both case classes and objects // def productArity = N // def productElement(i: Int): Any = i match { ... } // @@ -414,33 +415,21 @@ object desugar { } } - // Above MaxTupleArity we extend Product instead of ProductN, in this - // case we need to synthesise productElement & productArity. - def largeProductMeths = - if (arity > Definitions.MaxTupleArity) productElement :: productArity :: Nil - else Nil - if (isCaseClass) - largeProductMeths ::: copyMeths ::: enumTagMeths ::: productElemMeths.toList + productElement :: productArity :: copyMeths ::: enumTagMeths ::: productElemMeths.toList + else if (isCaseObject) + productArity :: productElement :: Nil else Nil } def anyRef = ref(defn.AnyRefAlias.typeRef) - def productConstr(n: Int) = { - val tycon = scalaDot((str.Product + n).toTypeName) - val targs = constrVparamss.head map (_.tpt) - if (targs.isEmpty) tycon else AppliedTypeTree(tycon, targs) - } - def product = - if (arity > Definitions.MaxTupleArity) scalaDot(str.Product.toTypeName) - else productConstr(arity) - // Case classes and case objects get Product/ProductN parents + // Case classes and case objects get Product parents var parents1 = parents if (isEnumCase && parents.isEmpty) parents1 = enumClassTypeRef :: Nil - if (mods.is(Case)) - parents1 = parents1 :+ product // TODO: This also adds Product0 to case objects. Do we want that? + if (isCaseClass | isCaseObject) + parents1 = parents1 :+ scalaDot(str.Product.toTypeName) if (isEnum) parents1 = parents1 :+ ref(defn.EnumType) @@ -499,7 +488,6 @@ object desugar { companionDefs(anyRef, Nil) else Nil - // For an implicit class C[Ts](p11: T11, ..., p1N: T1N) ... (pM1: TM1, .., pMN: TMN), the method // synthetic implicit C[Ts](p11: T11, ..., p1N: T1N) ... (pM1: TM1, ..., pMN: TMN): C[Ts] = // new C[Ts](p11, ..., p1N) ... (pM1, ..., pMN) = diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 4abaf3bc788c..d709235128a5 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -702,7 +702,6 @@ class Definitions { def FunctionClassPerRun = new PerRun[Array[Symbol]](implicit ctx => ImplementedFunctionType.map(_.symbol.asClass)) lazy val TupleType = mkArityArray("scala.Tuple", MaxTupleArity, 2) - lazy val ProductNType = mkArityArray("scala.Product", MaxTupleArity, 0) def FunctionClass(n: Int, isImplicit: Boolean = false)(implicit ctx: Context) = if (isImplicit) ctx.requiredClass("scala.ImplicitFunction" + n.toString) @@ -717,7 +716,6 @@ class Definitions { else FunctionClass(n, isImplicit).typeRef private lazy val TupleTypes: Set[TypeRef] = TupleType.toSet - private lazy val ProductTypes: Set[TypeRef] = ProductNType.toSet /** If `cls` is a class in the scala package, its name, otherwise EmptyTypeName */ def scalaClassName(cls: Symbol)(implicit ctx: Context): TypeName = diff --git a/tests/neg/t1843-variances.scala b/tests/neg/t1843-variances.scala index a6bdd686feec..65e3addf0b34 100644 --- a/tests/neg/t1843-variances.scala +++ b/tests/neg/t1843-variances.scala @@ -6,7 +6,7 @@ object Crash { trait UpdateType[A] - case class StateUpdate[+A](updateType : UpdateType[A], value : A) // error + case class StateUpdate[+A](updateType : UpdateType[A], value : A) // error // error case object IntegerUpdateType extends UpdateType[Integer] //However this method will cause a crash From 25b8b6e16aba7d1febf1c5f41ae03bbae5219afa Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Mon, 1 May 2017 18:17:24 +0200 Subject: [PATCH 2/5] Synthesize productElement in SyntheticMethods instead of Desugaring The implementation needed changes since Desugaring uses `untpd._` while SyntheticMethods uses `tpd._`. `productArity` is also removed from Desugaring, it's already implement in SyntheticMethods, so no changes there. --- .../src/dotty/tools/dotc/ast/Desugar.scala | 27 +------------ .../dotty/tools/dotc/core/Definitions.scala | 2 + .../dotc/transform/SyntheticMethods.scala | 39 ++++++++++++++++++- 3 files changed, 42 insertions(+), 26 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 63491b408dc6..34a4cec0d8fc 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -27,9 +27,7 @@ object desugar { /** Names of methods that are added unconditionally to case classes */ def isDesugaredCaseClassMethodName(name: Name)(implicit ctx: Context): Boolean = - name == nme.copy || - name == nme.productArity || - name.isSelectorName + name == nme.copy || name.isSelectorName // ----- DerivedTypeTrees ----------------------------------- @@ -361,31 +359,12 @@ object desugar { // pN: TN = pN: @uncheckedVariance)(moreParams) = // new C[...](p1, ..., pN)(moreParams) // - // To add to both case classes and objects - // def productArity = N - // def productElement(i: Int): Any = i match { ... } - // // Note: copy default parameters need @uncheckedVariance; see // neg/t1843-variances.scala for a test case. The test would give // two errors without @uncheckedVariance, one of them spurious. val caseClassMeths = { def syntheticProperty(name: TermName, rhs: Tree) = DefDef(name, Nil, Nil, TypeTree(), rhs).withMods(synthetic) - def productArity = syntheticProperty(nme.productArity, Literal(Constant(arity))) - def productElement = { - val param = makeSyntheticParameter(tpt = ref(defn.IntType)) - // case N => _${N + 1} - val cases = 0.until(arity).map { i => - CaseDef(Literal(Constant(i)), EmptyTree, Select(This(EmptyTypeIdent), nme.selectorName(i))) - } - val ioob = ref(defn.IndexOutOfBoundsException.typeRef) - val error = Throw(New(ioob, List(List(Select(refOfDef(param), nme.toString_))))) - // case _ => throw new IndexOutOfBoundsException(i.toString) - val defaultCase = CaseDef(untpd.Ident(nme.WILDCARD), EmptyTree, error) - val body = Match(refOfDef(param), (cases :+ defaultCase).toList) - DefDef(nme.productElement, Nil, List(List(param)), TypeTree(defn.AnyType), body) - .withMods(synthetic) - } def productElemMeths = { val caseParams = constrVparamss.head.toArray for (i <- 0 until arity if nme.selectorName(i) `ne` caseParams(i).name) @@ -416,9 +395,7 @@ object desugar { } if (isCaseClass) - productElement :: productArity :: copyMeths ::: enumTagMeths ::: productElemMeths.toList - else if (isCaseObject) - productArity :: productElement :: Nil + copyMeths ::: enumTagMeths ::: productElemMeths.toList else Nil } diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index d709235128a5..f0939cd416a0 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -525,6 +525,8 @@ class Definitions { def Product_canEqual(implicit ctx: Context) = Product_canEqualR.symbol lazy val Product_productArityR = ProductClass.requiredMethodRef(nme.productArity) def Product_productArity(implicit ctx: Context) = Product_productArityR.symbol + lazy val Product_productElementR = ProductClass.requiredMethodRef(nme.productElement) + def Product_productElement(implicit ctx: Context) = Product_productElementR.symbol lazy val Product_productPrefixR = ProductClass.requiredMethodRef(nme.productPrefix) def Product_productPrefix(implicit ctx: Context) = Product_productPrefixR.symbol lazy val LanguageModuleRef = ctx.requiredModule("scala.language") diff --git a/compiler/src/dotty/tools/dotc/transform/SyntheticMethods.scala b/compiler/src/dotty/tools/dotc/transform/SyntheticMethods.scala index 13af7e112739..e2fc98548fef 100644 --- a/compiler/src/dotty/tools/dotc/transform/SyntheticMethods.scala +++ b/compiler/src/dotty/tools/dotc/transform/SyntheticMethods.scala @@ -23,6 +23,7 @@ import scala.language.postfixOps * def hashCode(): Int * def canEqual(other: Any): Boolean * def toString(): String + * def productElement(i: Int): Any * def productArity: Int * def productPrefix: String * Special handling: @@ -44,7 +45,7 @@ class SyntheticMethods(thisTransformer: DenotTransformer) { if (myValueSymbols.isEmpty) { myValueSymbols = List(defn.Any_hashCode, defn.Any_equals) myCaseSymbols = myValueSymbols ++ List(defn.Any_toString, defn.Product_canEqual, - defn.Product_productArity, defn.Product_productPrefix) + defn.Product_productArity, defn.Product_productPrefix, defn.Product_productElement) } def valueSymbols(implicit ctx: Context) = { initSymbols; myValueSymbols } @@ -91,11 +92,47 @@ class SyntheticMethods(thisTransformer: DenotTransformer) { case nme.canEqual_ => vrefss => canEqualBody(vrefss.head.head) case nme.productArity => vrefss => Literal(Constant(accessors.length)) case nme.productPrefix => ownName + case nme.productElement => vrefss => productElementBody(accessors.length, vrefss.head.head) } ctx.log(s"adding $synthetic to $clazz at ${ctx.phase}") DefDef(synthetic, syntheticRHS(ctx.withOwner(synthetic))) } + /** The class + * + * ``` + * case class C(x: T, y: T) + * ``` + * + * gets the `productElement` method: + * + * ``` + * def productElement(index: Int): Any = index match { + * case 0 => this._1 + * case 1 => this._2 + * case _ => throw new IndexOutOfBoundsException(index.toString) + * } + * ``` + */ + def productElementBody(arity: Int, index: Tree)(implicit ctx: Context): Tree = { + val ioob = defn.IndexOutOfBoundsException.typeRef + // That's not ioob.typeSymbol.primaryConstructor, this is the other one + // that takes a String argument. + val constructor = ioob.typeSymbol.info.decls.toList.tail.head.asTerm + val stringIndex = Apply(Select(index, nme.toString_), Nil) + val error = Throw(New(ioob, constructor, List(stringIndex))) + + // case _ => throw new IndexOutOfBoundsException(i.toString) + val defaultCase = CaseDef(Underscore(defn.IntType), EmptyTree, error) + + // case N => _${N + 1} + val cases = 0.until(arity).map { i => + CaseDef(Literal(Constant(i)), EmptyTree, Select(This(clazz), nme.selectorName(i))) + } + + Match(index, (cases :+ defaultCase).toList) + } + /** The class * * case class C(x: T, y: U) From 70ac7c3dd35ef664063c05ae00907ac99aecc6d3 Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Mon, 1 May 2017 18:20:41 +0200 Subject: [PATCH 3/5] Reformat scaladoc in SyntheticMethods --- .../dotc/transform/SyntheticMethods.scala | 70 ++++++++++++------- 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/SyntheticMethods.scala b/compiler/src/dotty/tools/dotc/transform/SyntheticMethods.scala index e2fc98548fef..2550e0639565 100644 --- a/compiler/src/dotty/tools/dotc/transform/SyntheticMethods.scala +++ b/compiler/src/dotty/tools/dotc/transform/SyntheticMethods.scala @@ -17,6 +17,7 @@ import scala.language.postfixOps /** Synthetic method implementations for case classes, case objects, * and value classes. + * * Selectively added to case classes/objects, unless a non-default * implementation already exists: * def equals(other: Any): Boolean @@ -26,12 +27,12 @@ import scala.language.postfixOps * def productElement(i: Int): Any * def productArity: Int * def productPrefix: String + * * Special handling: * protected def readResolve(): AnyRef * * Selectively added to value classes, unless a non-default * implementation already exists: - * * def equals(other: Any): Boolean * def hashCode(): Int */ @@ -51,8 +52,7 @@ class SyntheticMethods(thisTransformer: DenotTransformer) { def valueSymbols(implicit ctx: Context) = { initSymbols; myValueSymbols } def caseSymbols(implicit ctx: Context) = { initSymbols; myCaseSymbols } - /** The synthetic methods of the case or value class `clazz`. - */ + /** The synthetic methods of the case or value class `clazz`. */ def syntheticMethods(clazz: ClassSymbol)(implicit ctx: Context): List[Tree] = { val clazzType = clazz.typeRef lazy val accessors = @@ -135,18 +135,22 @@ class SyntheticMethods(thisTransformer: DenotTransformer) { /** The class * - * case class C(x: T, y: U) + * ``` + * case class C(x: T, y: U) + * ``` * - * gets the equals method: + * gets the `equals` method: * - * def equals(that: Any): Boolean = - * (this eq that) || { - * that match { - * case x$0 @ (_: C) => this.x == this$0.x && this.y == x$0.y - * case _ => false - * } + * ``` + * def equals(that: Any): Boolean = + * (this eq that) || { + * that match { + * case x$0 @ (_: C) => this.x == this$0.x && this.y == x$0.y + * case _ => false + * } + * ``` * - * If C is a value class the initial `eq` test is omitted. + * If `C` is a value class the initial `eq` test is omitted. */ def equalsBody(that: Tree)(implicit ctx: Context): Tree = { val thatAsClazz = ctx.newSymbol(ctx.owner, nme.x_0, Synthetic, clazzType, coord = ctx.owner.pos) // x$0 @@ -168,11 +172,15 @@ class SyntheticMethods(thisTransformer: DenotTransformer) { /** The class * + * ``` * class C(x: T) extends AnyVal + * ``` * - * gets the hashCode method: + * gets the `hashCode` method: * - * def hashCode: Int = x.hashCode() + * ``` + * def hashCode: Int = x.hashCode() + * ``` */ def valueHashCodeBody(implicit ctx: Context): Tree = { assert(accessors.length == 1) @@ -181,17 +189,21 @@ class SyntheticMethods(thisTransformer: DenotTransformer) { /** The class * - * package p - * case class C(x: T, y: T) + * ``` + * package p + * case class C(x: T, y: T) + * ``` * - * gets the hashCode method: + * gets the `hashCode` method: * - * def hashCode: Int = { - * var acc: Int = "p.C".hashCode // constant folded - * acc = Statics.mix(acc, x); - * acc = Statics.mix(acc, Statics.this.anyHash(y)); - * Statics.finalizeHash(acc, 2) - * } + * ``` + * def hashCode: Int = { + * var acc: Int = "p.C".hashCode // constant folded + * acc = Statics.mix(acc, x); + * acc = Statics.mix(acc, Statics.this.anyHash(y)); + * Statics.finalizeHash(acc, 2) + * } + * ``` */ def caseHashCodeBody(implicit ctx: Context): Tree = { val acc = ctx.newSymbol(ctx.owner, "acc".toTermName, Mutable | Synthetic, defn.IntType, coord = ctx.owner.pos) @@ -202,7 +214,7 @@ class SyntheticMethods(thisTransformer: DenotTransformer) { Block(accDef :: mixes, finish) } - /** The hashCode implementation for given symbol `sym`. */ + /** The `hashCode` implementation for given symbol `sym`. */ def hashImpl(sym: Symbol)(implicit ctx: Context): Tree = defn.scalaClassName(sym.info.finalResultType) match { case tpnme.Unit | tpnme.Null => Literal(Constant(0)) @@ -217,11 +229,15 @@ class SyntheticMethods(thisTransformer: DenotTransformer) { /** The class * - * case class C(...) + * ``` + * case class C(...) + * ``` * - * gets the canEqual method + * gets the `canEqual` method * - * def canEqual(that: Any) = that.isInstanceOf[C] + * ``` + * def canEqual(that: Any) = that.isInstanceOf[C] + * ``` */ def canEqualBody(that: Tree): Tree = that.isInstance(clazzType) From df39a40a78adb94ee2de2789408b1b69a376060b Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Mon, 1 May 2017 18:21:20 +0200 Subject: [PATCH 4/5] Test that product{Arity, Element} are synthesised on demend --- tests/run/i2314.scala | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 tests/run/i2314.scala diff --git a/tests/run/i2314.scala b/tests/run/i2314.scala new file mode 100644 index 000000000000..37e52a1368c5 --- /dev/null +++ b/tests/run/i2314.scala @@ -0,0 +1,37 @@ +case class A(i: Int, s: String) + +case class B(i: Int, s: String) { + // No override, these methods will be added by SyntheticMethods only if + // there are not user defined. + def productArity = -1 + def productElement(i: Int): Any = None +} + +object Test { + def main(args: Array[String]): Unit = { + val a = A(1, "s") + assert(a.productArity == 2) + assert(a.productElement(0) == 1) + assert(a.productElement(1) == "s") + + try { + a.productElement(-1) + ??? + } catch { + case e: IndexOutOfBoundsException => + assert(e.getMessage == "-1") + } + try { + a.productElement(2) + ??? + } catch { + case e: IndexOutOfBoundsException => + assert(e.getMessage == "2") + } + + val b = B(1, "s") + assert(b.productArity == -1) + assert(b.productElement(0) == None) + assert(b.productElement(1) == None) + } +} From c7de6a4fc46417a466a79e5cf2931c9fcd063fd0 Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Tue, 2 May 2017 10:02:01 +0200 Subject: [PATCH 5/5] Select ioob constructor by type instead of position in declarations --- .../dotty/tools/dotc/transform/SyntheticMethods.scala | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/SyntheticMethods.scala b/compiler/src/dotty/tools/dotc/transform/SyntheticMethods.scala index 2550e0639565..d37e47d49ba6 100644 --- a/compiler/src/dotty/tools/dotc/transform/SyntheticMethods.scala +++ b/compiler/src/dotty/tools/dotc/transform/SyntheticMethods.scala @@ -116,9 +116,12 @@ class SyntheticMethods(thisTransformer: DenotTransformer) { */ def productElementBody(arity: Int, index: Tree)(implicit ctx: Context): Tree = { val ioob = defn.IndexOutOfBoundsException.typeRef - // That's not ioob.typeSymbol.primaryConstructor, this is the other one - // that takes a String argument. - val constructor = ioob.typeSymbol.info.decls.toList.tail.head.asTerm + // Second constructor of ioob that takes a String argument + def filterStringConstructor(s: Symbol): Boolean = s.info match { + case m: MethodType if s.isConstructor => m.paramInfos == List(defn.StringType) + case _ => false + } + val constructor = ioob.typeSymbol.info.decls.find(filterStringConstructor _).asTerm val stringIndex = Apply(Select(index, nme.toString_), Nil) val error = Throw(New(ioob, constructor, List(stringIndex)))