From 7602f2ebc0fbd0e1b51aa8d9d9a9e71607a06dd6 Mon Sep 17 00:00:00 2001 From: Iulian Dragos Date: Mon, 21 Nov 2016 13:27:16 +0100 Subject: SI-10071 Separate compilation for varargs methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make sure that methods annotated with varargs are properly mixed-in. This commit splits the transformation into an info transformer (that works on all symbols, whether they come from source or binary) and a tree transformer. The gist of this is that the symbol-creation part of the code was moved to the UnCurry info transformer, while tree operations remained in the tree transformer. The newly created symbol is attached to the original method so that the tree transformer can still retrieve the symbol. A few fall outs: - I removed a local map that was identical to TypeParamsVarargsAttachment - moved the said attachment to StdAttachments so it’s visible between reflect.internal and nsc.transform - a couple more comments in UnCurry to honour the boy-scout rule --- .../scala/tools/nsc/transform/Erasure.scala | 1 - .../scala/tools/nsc/transform/UnCurry.scala | 95 ++++++++-------------- .../scala/reflect/internal/StdAttachments.scala | 3 + .../scala/reflect/internal/transform/UnCurry.scala | 82 ++++++++++++++++++- .../scala/reflect/runtime/JavaUniverseForce.scala | 2 + 5 files changed, 119 insertions(+), 64 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala index 25475515aa..92accaf9dd 100644 --- a/src/compiler/scala/tools/nsc/transform/Erasure.scala +++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala @@ -1278,5 +1278,4 @@ abstract class Erasure extends InfoTransform } private class TypeRefAttachment(val tpe: TypeRef) - class TypeParamVarargsAttachment(val typeParamRef: Type) } diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala index d8fa7b58e8..ea3c7da014 100644 --- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala +++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala @@ -27,6 +27,8 @@ import scala.reflect.internal.util.ListOfNil * - for every repeated Scala parameter `x: T*' --> x: Seq[T]. * - for every repeated Java parameter `x: T...' --> x: Array[T], except: * if T is an unbounded abstract type, replace --> x: Array[Object] + * - for every method defining repeated parameters annotated with @varargs, generate + * a synthetic Java-style vararg method * - for every argument list that corresponds to a repeated Scala parameter * (a_1, ..., a_n) => (Seq(a_1, ..., a_n)) * - for every argument list that corresponds to a repeated Java parameter @@ -44,6 +46,8 @@ import scala.reflect.internal.util.ListOfNil * def liftedTry$1 = try { x_i } catch { .. } * meth(x_1, .., liftedTry$1(), .. ) * } + * - remove calls to elidable methods and replace their bodies with NOPs when elide-below + * requires it */ /* */ abstract class UnCurry extends InfoTransform @@ -577,7 +581,13 @@ abstract class UnCurry extends InfoTransform case None => literalRhsIfConst } ) - addJavaVarargsForwarders(dd, flatdd) + // Only class members can reasonably be called from Java due to name mangling. + // Additionally, the Uncurry info transformer only adds a forwarder symbol to class members, + // since the other symbols are not part of the ClassInfoType (see reflect.internal.transform.UnCurry) + if (dd.symbol.owner.isClass) + addJavaVarargsForwarders(dd, flatdd) + else + flatdd case tree: Try => if (tree.catches exists (cd => !treeInfo.isCatchCase(cd))) @@ -739,68 +749,32 @@ abstract class UnCurry extends InfoTransform if (!hasRepeated) reporter.error(dd.symbol.pos, "A method without repeated parameters cannot be annotated with the `varargs` annotation.") } - /* Called during post transform, after the method argument lists have been flattened. - * It looks for the method in the `repeatedParams` map, and generates a Java-style + /** + * Called during post transform, after the method argument lists have been flattened. + * It looks for the forwarder symbol in the symbol attachments and generates a Java-style * varargs forwarder. + * + * @note The Java-style varargs method symbol is generated in the Uncurry info transformer. If the + * symbol can't be found this method reports a warning and carries on. + * @see [[scala.reflect.internal.transform.UnCurry]] */ private def addJavaVarargsForwarders(dd: DefDef, flatdd: DefDef): DefDef = { if (!dd.symbol.hasAnnotation(VarargsClass) || !enteringUncurry(mexists(dd.symbol.paramss)(sym => definitions.isRepeatedParamType(sym.tpe)))) return flatdd - val forwSym = currentClass.newMethod(dd.name.toTermName, dd.pos, VARARGS | SYNTHETIC | flatdd.symbol.flags & ~DEFERRED) - - val isRepeated = enteringUncurry(dd.symbol.info.paramss.flatten.map(sym => definitions.isRepeatedParamType(sym.tpe))) - - val oldPs = flatdd.symbol.paramss.head - - // see comment in method toArrayType below - val arrayTypesMappedToObject = mutable.Map.empty[Symbol, Type] - - val forwTpe = { - val (oldTps, tps) = dd.symbol.tpe match { - case PolyType(oldTps, _) => - val newTps = oldTps.map(_.cloneSymbol(forwSym)) - (oldTps, newTps) - - case _ => (Nil, Nil) - } - - def toArrayType(tp: Type, newParam: Symbol): Type = { - val arg = elementType(SeqClass, tp) - val elem = if (arg.typeSymbol.isTypeParameterOrSkolem && !(arg <:< AnyRefTpe)) { - // To prevent generation of an `Object` parameter from `Array[T]` parameter later - // as this would crash the Java compiler which expects an `Object[]` array for varargs - // e.g. def foo[T](a: Int, b: T*) - // becomes def foo[T](a: Int, b: Array[Object]) - // instead of def foo[T](a: Int, b: Array[T]) ===> def foo[T](a: Int, b: Object) - // - // In order for the forwarder method to type check we need to insert a cast: - // def foo'[T'](a: Int, b: Array[Object]) = foo[T'](a, wrapRefArray(b).asInstanceOf[Seq[T']]) - // The target element type for that cast (T') is stored in the `arrayTypesMappedToObject` map. - val originalArg = arg.substSym(oldTps, tps) - arrayTypesMappedToObject(newParam) = originalArg - // Store the type parameter that was replaced by Object to emit the correct generic signature - newParam.updateAttachment(new erasure.TypeParamVarargsAttachment(originalArg)) - ObjectTpe - } else - arg - arrayType(elem) + val forwSym: Symbol = { + currentClass.info // make sure the info is up to date, so the varargs forwarder symbol has been generated + flatdd.symbol.attachments.get[VarargsSymbolAttachment] match { + case Some(VarargsSymbolAttachment(sym)) => sym + case None => + reporter.warning(dd.pos, s"Could not generate Java varargs forwarder for ${flatdd.symbol}. Please file a bug.") + return flatdd } - - val ps = map2(oldPs, isRepeated)((oldParam, isRep) => { - val newParam = oldParam.cloneSymbol(forwSym) - val tp = if (isRep) toArrayType(oldParam.tpe, newParam) else oldParam.tpe - newParam.setInfo(tp) - }) - - val resTp = dd.symbol.tpe_*.finalResultType.substSym(oldPs, ps) - val mt = MethodType(ps, resTp) - val r = if (tps.isEmpty) mt else PolyType(tps, mt) - r.substSym(oldTps, tps) } - forwSym.setInfo(forwTpe) - val newPs = forwTpe.params + val newPs = forwSym.tpe.params + val isRepeated = enteringUncurry(dd.symbol.info.paramss.flatten.map(sym => definitions.isRepeatedParamType(sym.tpe))) + val oldPs = flatdd.symbol.paramss.head val theTyper = typer.atOwner(dd, currentClass) val forwTree = theTyper.typedPos(dd.pos) { @@ -809,8 +783,8 @@ abstract class UnCurry extends InfoTransform else { val parTp = elementType(ArrayClass, param.tpe) val wrap = gen.mkWrapArray(Ident(param), parTp) - arrayTypesMappedToObject.get(param) match { - case Some(tp) => gen.mkCast(wrap, seqType(tp)) + param.attachments.get[TypeParamVarargsAttachment] match { + case Some(TypeParamVarargsAttachment(tp)) => gen.mkCast(wrap, seqType(tp)) case _ => wrap } } @@ -821,13 +795,12 @@ abstract class UnCurry extends InfoTransform } // check if the method with that name and those arguments already exists in the template - currentClass.info.member(forwSym.name).alternatives.find(s => s != forwSym && s.tpe.matches(forwSym.tpe)) match { - case Some(s) => reporter.error(dd.symbol.pos, - "A method with a varargs annotation produces a forwarder method with the same signature " - + s.tpe + " as an existing method.") + enteringUncurry(currentClass.info.member(forwSym.name).alternatives.find(s => s != forwSym && s.tpe.matches(forwSym.tpe))) match { + case Some(s) => + reporter.error(dd.symbol.pos, + s"A method with a varargs annotation produces a forwarder method with the same signature ${s.tpe} as an existing method.") case None => // enter symbol into scope - currentClass.info.decls enter forwSym addNewMember(forwTree) } diff --git a/src/reflect/scala/reflect/internal/StdAttachments.scala b/src/reflect/scala/reflect/internal/StdAttachments.scala index 78f360409d..fd8f51cfb1 100644 --- a/src/reflect/scala/reflect/internal/StdAttachments.scala +++ b/src/reflect/scala/reflect/internal/StdAttachments.scala @@ -78,4 +78,7 @@ trait StdAttachments { case object OuterArgCanBeElided extends PlainAttachment case object UseInvokeSpecial extends PlainAttachment + + /** An attachment carrying information between uncurry and erasure */ + case class TypeParamVarargsAttachment(val typeParamRef: Type) } diff --git a/src/reflect/scala/reflect/internal/transform/UnCurry.scala b/src/reflect/scala/reflect/internal/transform/UnCurry.scala index a50084f40d..222f25440e 100644 --- a/src/reflect/scala/reflect/internal/transform/UnCurry.scala +++ b/src/reflect/scala/reflect/internal/transform/UnCurry.scala @@ -4,6 +4,7 @@ package internal package transform import Flags._ +import scala.collection.mutable trait UnCurry { @@ -11,6 +12,12 @@ trait UnCurry { import global._ import definitions._ + /** + * The synthetic Java vararg method symbol corresponding to a Scala vararg method + * annotated with @varargs. + */ + case class VarargsSymbolAttachment(varargMethod: Symbol) + /** Note: changing tp.normalize to tp.dealias in this method leads to a single * test failure: run/t5688.scala, where instead of the expected output * Vector(ta, tb, tab) @@ -67,8 +74,25 @@ trait UnCurry { tp match { case ClassInfoType(parents, decls, clazz) => val parents1 = parents mapConserve uncurry - if (parents1 eq parents) tp - else ClassInfoType(parents1, decls, clazz) // @MAT normalize in decls?? + val varargOverloads = mutable.ListBuffer.empty[Symbol] + + // Not using `hasAnnotation` here because of dreaded cyclic reference errors: + // it may happen that VarargsClass has not been initialized yet and we get here + // while processing one of its superclasses (such as java.lang.Object). Since we + // don't need the more precise `matches` semantics, we only check the symbol, which + // is anyway faster and safer + for (decl <- decls if decl.annotations.exists(_.symbol == VarargsClass)) { + if (mexists(decl.paramss)(sym => definitions.isRepeatedParamType(sym.tpe))) { + val forwarderSym = varargForwarderSym(clazz, decl, exitingPhase(phase)(decl.info)) + varargOverloads += forwarderSym + } + } + if ((parents1 eq parents) && varargOverloads.isEmpty) tp + else { + val newDecls = decls.cloneScope + varargOverloads.foreach(newDecls.enter) + ClassInfoType(parents1, newDecls, clazz) + } // @MAT normalize in decls?? case PolyType(_, _) => mapOver(tp) case _ => @@ -77,6 +101,60 @@ trait UnCurry { } } + private def varargForwarderSym(currentClass: Symbol, origSym: Symbol, newInfo: Type): Symbol = { + val forwSym = currentClass.newMethod(origSym.name.toTermName, origSym.pos, VARARGS | SYNTHETIC | origSym.flags & ~DEFERRED) + + // we are using `origSym.info`, which contains the type *before* the transformation + // so we still see repeated parameter types (uncurry replaces them with Seq) + val isRepeated = origSym.info.paramss.flatten.map(sym => definitions.isRepeatedParamType(sym.tpe)) + val oldPs = newInfo.paramss.head + + val forwTpe = { + val (oldTps, tps) = newInfo match { + case PolyType(oldTps, _) => + val newTps = oldTps.map(_.cloneSymbol(forwSym)) + (oldTps, newTps) + + case _ => (Nil, Nil) + } + + def toArrayType(tp: Type, newParam: Symbol): Type = { + val arg = elementType(SeqClass, tp) + val elem = if (arg.typeSymbol.isTypeParameterOrSkolem && !(arg <:< AnyRefTpe)) { + // To prevent generation of an `Object` parameter from `Array[T]` parameter later + // as this would crash the Java compiler which expects an `Object[]` array for varargs + // e.g. def foo[T](a: Int, b: T*) + // becomes def foo[T](a: Int, b: Array[Object]) + // instead of def foo[T](a: Int, b: Array[T]) ===> def foo[T](a: Int, b: Object) + // + // In order for the forwarder method to type check we need to insert a cast: + // def foo'[T'](a: Int, b: Array[Object]) = foo[T'](a, wrapRefArray(b).asInstanceOf[Seq[T']]) + // The target element type for that cast (T') is stored in the TypeParamVarargsAttachment + val originalArg = arg.substSym(oldTps, tps) + // Store the type parameter that was replaced by Object to emit the correct generic signature + newParam.updateAttachment(new TypeParamVarargsAttachment(originalArg)) + ObjectTpe + } else + arg + arrayType(elem) + } + + val ps = map2(oldPs, isRepeated)((oldParam, isRep) => { + val newParam = oldParam.cloneSymbol(forwSym) + val tp = if (isRep) toArrayType(oldParam.tpe, newParam) else oldParam.tpe + newParam.setInfo(tp) + }) + + val resTp = newInfo.finalResultType.substSym(oldPs, ps) + val mt = MethodType(ps, resTp) + val r = if (tps.isEmpty) mt else PolyType(tps, mt) + r.substSym(oldTps, tps) + } + + origSym.updateAttachment(VarargsSymbolAttachment(forwSym)) + forwSym.setInfo(forwTpe) + } + /** - return symbol's transformed type, * - if symbol is a def parameter with transformed type T, return () => T * diff --git a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala index b74ccb9177..dbafbfc6ba 100644 --- a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala +++ b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala @@ -47,6 +47,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse => this.InlineCallsiteAttachment this.OuterArgCanBeElided this.UseInvokeSpecial + this.TypeParamVarargsAttachment this.noPrint this.typeDebug this.Range @@ -458,6 +459,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse => definitions.ScalaValueClassesNoUnit definitions.ScalaValueClasses + uncurry.VarargsSymbolAttachment uncurry.DesugaredParameterType erasure.GenericArray erasure.scalaErasure -- cgit v1.2.3 From 6e719afe609a7447d0f9717ff9548818d3b94f5d Mon Sep 17 00:00:00 2001 From: Iulian Dragos Date: Thu, 24 Nov 2016 14:51:09 +0100 Subject: Don’t run the uncurry info transformer on Java symbols. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/reflect/scala/reflect/internal/transform/UnCurry.scala | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/reflect/scala/reflect/internal/transform/UnCurry.scala b/src/reflect/scala/reflect/internal/transform/UnCurry.scala index 222f25440e..e20f1f04f6 100644 --- a/src/reflect/scala/reflect/internal/transform/UnCurry.scala +++ b/src/reflect/scala/reflect/internal/transform/UnCurry.scala @@ -72,7 +72,7 @@ trait UnCurry { def apply(tp0: Type): Type = { val tp = expandAlias(tp0) tp match { - case ClassInfoType(parents, decls, clazz) => + case ClassInfoType(parents, decls, clazz) if !clazz.isJavaDefined => val parents1 = parents mapConserve uncurry val varargOverloads = mutable.ListBuffer.empty[Symbol] @@ -83,8 +83,7 @@ trait UnCurry { // is anyway faster and safer for (decl <- decls if decl.annotations.exists(_.symbol == VarargsClass)) { if (mexists(decl.paramss)(sym => definitions.isRepeatedParamType(sym.tpe))) { - val forwarderSym = varargForwarderSym(clazz, decl, exitingPhase(phase)(decl.info)) - varargOverloads += forwarderSym + varargOverloads += varargForwarderSym(clazz, decl, exitingPhase(phase)(decl.info)) } } if ((parents1 eq parents) && varargOverloads.isEmpty) tp @@ -93,8 +92,10 @@ trait UnCurry { varargOverloads.foreach(newDecls.enter) ClassInfoType(parents1, newDecls, clazz) } // @MAT normalize in decls?? + case PolyType(_, _) => mapOver(tp) + case _ => tp } -- cgit v1.2.3 From 0fba8820d9773c9c718384d696032110d5c74b72 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 25 Nov 2016 08:28:57 +1000 Subject: Simplify creation of varargs forwarder symbol Cloning the original symbol in its entirety, rather than cloning its type/value parameters individually. `cloneSymbol` takes care of all the tricky substitutions for us! --- .../scala/reflect/internal/transform/UnCurry.scala | 68 +++++++++------------- 1 file changed, 26 insertions(+), 42 deletions(-) (limited to 'src') diff --git a/src/reflect/scala/reflect/internal/transform/UnCurry.scala b/src/reflect/scala/reflect/internal/transform/UnCurry.scala index e20f1f04f6..3918723b5c 100644 --- a/src/reflect/scala/reflect/internal/transform/UnCurry.scala +++ b/src/reflect/scala/reflect/internal/transform/UnCurry.scala @@ -103,57 +103,41 @@ trait UnCurry { } private def varargForwarderSym(currentClass: Symbol, origSym: Symbol, newInfo: Type): Symbol = { - val forwSym = currentClass.newMethod(origSym.name.toTermName, origSym.pos, VARARGS | SYNTHETIC | origSym.flags & ~DEFERRED) + val forwSym = origSym.cloneSymbol(currentClass, VARARGS | SYNTHETIC | origSym.flags & ~DEFERRED, origSym.name.toTermName).withoutAnnotations // we are using `origSym.info`, which contains the type *before* the transformation // so we still see repeated parameter types (uncurry replaces them with Seq) val isRepeated = origSym.info.paramss.flatten.map(sym => definitions.isRepeatedParamType(sym.tpe)) val oldPs = newInfo.paramss.head + def toArrayType(tp: Type, newParam: Symbol): Type = { + val arg = elementType(SeqClass, tp) + val elem = if (arg.typeSymbol.isTypeParameterOrSkolem && !(arg <:< AnyRefTpe)) { + // To prevent generation of an `Object` parameter from `Array[T]` parameter later + // as this would crash the Java compiler which expects an `Object[]` array for varargs + // e.g. def foo[T](a: Int, b: T*) + // becomes def foo[T](a: Int, b: Array[Object]) + // instead of def foo[T](a: Int, b: Array[T]) ===> def foo[T](a: Int, b: Object) + // + // In order for the forwarder method to type check we need to insert a cast: + // def foo'[T'](a: Int, b: Array[Object]) = foo[T'](a, wrapRefArray(b).asInstanceOf[Seq[T']]) + // The target element type for that cast (T') is stored in the TypeParamVarargsAttachment +// val originalArg = arg.substSym(oldTps, tps) + // Store the type parameter that was replaced by Object to emit the correct generic signature + newParam.updateAttachment(new TypeParamVarargsAttachment(arg)) + ObjectTpe + } else + arg + arrayType(elem) + } - val forwTpe = { - val (oldTps, tps) = newInfo match { - case PolyType(oldTps, _) => - val newTps = oldTps.map(_.cloneSymbol(forwSym)) - (oldTps, newTps) - - case _ => (Nil, Nil) - } - - def toArrayType(tp: Type, newParam: Symbol): Type = { - val arg = elementType(SeqClass, tp) - val elem = if (arg.typeSymbol.isTypeParameterOrSkolem && !(arg <:< AnyRefTpe)) { - // To prevent generation of an `Object` parameter from `Array[T]` parameter later - // as this would crash the Java compiler which expects an `Object[]` array for varargs - // e.g. def foo[T](a: Int, b: T*) - // becomes def foo[T](a: Int, b: Array[Object]) - // instead of def foo[T](a: Int, b: Array[T]) ===> def foo[T](a: Int, b: Object) - // - // In order for the forwarder method to type check we need to insert a cast: - // def foo'[T'](a: Int, b: Array[Object]) = foo[T'](a, wrapRefArray(b).asInstanceOf[Seq[T']]) - // The target element type for that cast (T') is stored in the TypeParamVarargsAttachment - val originalArg = arg.substSym(oldTps, tps) - // Store the type parameter that was replaced by Object to emit the correct generic signature - newParam.updateAttachment(new TypeParamVarargsAttachment(originalArg)) - ObjectTpe - } else - arg - arrayType(elem) + foreach2(forwSym.paramss.flatten, isRepeated)((p, isRep) => + if (isRep) { + p.setInfo(toArrayType(p.info, p)) } - - val ps = map2(oldPs, isRepeated)((oldParam, isRep) => { - val newParam = oldParam.cloneSymbol(forwSym) - val tp = if (isRep) toArrayType(oldParam.tpe, newParam) else oldParam.tpe - newParam.setInfo(tp) - }) - - val resTp = newInfo.finalResultType.substSym(oldPs, ps) - val mt = MethodType(ps, resTp) - val r = if (tps.isEmpty) mt else PolyType(tps, mt) - r.substSym(oldTps, tps) - } + ) origSym.updateAttachment(VarargsSymbolAttachment(forwSym)) - forwSym.setInfo(forwTpe) + forwSym } /** - return symbol's transformed type, -- cgit v1.2.3