From 106749e7b3447249a62509dd932e6f7911ddea56 Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Fri, 7 Dec 2018 17:03:48 +0100 Subject: [PATCH 1/2] Fix #4785: Change typing rule for wildcard star args We typecheck them without expected type to avoid relying on the implicit conversion from Array to Seq defined in Predef. ElimRepeated does the necessary adaptations (i.e. seqToArray and arrayToSeq conversions). --- .../tools/dotc/core/TypeApplications.scala | 2 +- .../tools/dotc/transform/ElimRepeated.scala | 17 +++++++++++----- .../dotty/tools/dotc/typer/TypeAssigner.scala | 8 ++++++-- .../src/dotty/tools/dotc/typer/Typer.scala | 20 +++++++++++++++++-- tests/neg/repeatedArgs213.scala | 4 ++-- tests/pos/i4785.scala | 15 ++++++++++++++ 6 files changed, 54 insertions(+), 12 deletions(-) create mode 100644 tests/pos/i4785.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeApplications.scala b/compiler/src/dotty/tools/dotc/core/TypeApplications.scala index b2517f39655c..c38b777a3b20 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeApplications.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeApplications.scala @@ -497,7 +497,7 @@ class TypeApplications(val self: Type) extends AnyVal { } /** The element type of a sequence or array */ - def elemType(implicit ctx: Context): Type = self match { + def elemType(implicit ctx: Context): Type = self.widenDealias match { case defn.ArrayOf(elemtp) => elemtp case JavaArrayType(elemtp) => elemtp case _ => self.baseType(defn.SeqClass).argInfos.headOption.getOrElse(NoType) diff --git a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala index 1367538630ba..6c7e452e3d32 100644 --- a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala +++ b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala @@ -78,9 +78,14 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => val args1 = tree.args.zipWithConserve(formals) { (arg, formal) => arg match { case arg: Typed if isWildcardStarArg(arg) => - if (tree.fun.symbol.is(JavaDefined) && arg.expr.tpe.derivesFrom(defn.SeqClass)) + val isJavaDefined = tree.fun.symbol.is(JavaDefined) + val tpe = arg.expr.tpe + if (isJavaDefined && tpe.derivesFrom(defn.SeqClass)) seqToArray(arg.expr, formal.underlyingIfRepeated(isJava = true)) - else arg.expr + else if (!isJavaDefined && tpe.derivesFrom(defn.ArrayClass)) + arrayToSeq(arg.expr) + else + arg.expr case arg => arg } } @@ -91,12 +96,10 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => private def seqToArray(tree: Tree, pt: Type)(implicit ctx: Context): Tree = tree match { case SeqLiteral(elems, elemtpt) => JavaSeqLiteral(elems, elemtpt) - case app@Apply(fun, args) if defn.WrapArrayMethods().contains(fun.symbol) => // rewrite a call to `wrapXArray(arr)` to `arr` - args.head case _ => val elemType = tree.tpe.elemType var elemClass = elemType.classSymbol - if (defn.NotRuntimeClasses contains elemClass) elemClass = defn.ObjectClass + if (defn.NotRuntimeClasses.contains(elemClass)) elemClass = defn.ObjectClass ref(defn.DottyArraysModule) .select(nme.seqToArray) .appliedToType(elemType) @@ -105,6 +108,10 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => // Because of phantomclasses, the Java array's type might not conform to the return type } + /** Convert Java array argument to Scala Seq */ + private def arrayToSeq(tree: Tree)(implicit ctx: Context): Tree = + tpd.wrapArray(tree, tree.tpe.elemType) + override def transformTypeApply(tree: TypeApply)(implicit ctx: Context): Tree = transformTypeOfTree(tree) diff --git a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala index 1297c13dd108..17fc86a6c571 100644 --- a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -153,8 +153,12 @@ trait TypeAssigner { if (!sym.is(SyntheticOrPrivate) && sym.owner.isClass) checkNoPrivateLeaks(sym, pos) else sym.info - def seqToRepeated(tree: Tree)(implicit ctx: Context): Tree = - Typed(tree, TypeTree(tree.tpe.widen.translateParameterized(defn.SeqClass, defn.RepeatedParamClass))) + private def toRepeated(tree: Tree, from: ClassSymbol)(implicit ctx: Context): Tree = + Typed(tree, TypeTree(tree.tpe.widen.translateParameterized(from, defn.RepeatedParamClass))) + + def seqToRepeated(tree: Tree)(implicit ctx: Context): Tree = toRepeated(tree, defn.SeqClass) + + def arrayToRepeated(tree: Tree)(implicit ctx: Context): Tree = toRepeated(tree, defn.ArrayClass) /** A denotation exists really if it exists and does not point to a stale symbol. */ final def reallyExists(denot: Denotation)(implicit ctx: Context): Boolean = try diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index ebf8edc6ed2a..5dfbf153903f 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -546,6 +546,7 @@ class Typer extends Namer } case _ => ifExpr } + def ascription(tpt: Tree, isWildcard: Boolean) = { val underlyingTreeTpe = if (isRepeatedParamType(tpt)) TypeTree(defn.SeqType.appliedTo(pt :: Nil)) @@ -557,11 +558,26 @@ class Typer extends Namer else typed(tree.expr, tpt.tpe.widenSkolem) assignType(cpy.Typed(tree)(expr1, tpt), underlyingTreeTpe) } - if (untpd.isWildcardStarArg(tree)) + + if (untpd.isWildcardStarArg(tree)) { + def typedWildcardStarArgExpr = { + val tpdExpr = typedExpr(tree.expr) + tpdExpr.tpe.widenDealias match { + case defn.ArrayOf(_) => + val starType = defn.ArrayType.appliedTo(WildcardType) + val exprAdapted = adapt(tpdExpr, starType) + arrayToRepeated(exprAdapted) + case _ => + val starType = defn.SeqType.appliedTo(defn.AnyType) + val exprAdapted = adapt(tpdExpr, starType) + seqToRepeated(exprAdapted) + } + } cases( ifPat = ascription(TypeTree(defn.RepeatedParamType.appliedTo(pt)), isWildcard = true), - ifExpr = seqToRepeated(typedExpr(tree.expr, defn.SeqType.appliedTo(defn.AnyType))), + ifExpr = typedWildcardStarArgExpr, wildName = nme.WILDCARD_STAR) + } else { def typedTpt = checkSimpleKinded(typedType(tree.tpt)) def handlePattern: Tree = { diff --git a/tests/neg/repeatedArgs213.scala b/tests/neg/repeatedArgs213.scala index e650a371ecc3..dad19f759086 100644 --- a/tests/neg/repeatedArgs213.scala +++ b/tests/neg/repeatedArgs213.scala @@ -33,11 +33,11 @@ class repeatedArgs { bar("a", "b", "c") bar(xs: _*) bar(ys: _*) // error: immutable.Seq expected, found Seq - bar(zs: _*) // error: immutable.Seq expected, found Array + bar(zs: _*) // old-error: Remove (compiler generated) Array to Seq convertion in 2.13? Paths.get("Hello", "World") Paths.get("Hello", xs: _*) Paths.get("Hello", ys: _*) // error: immutable.Seq expected, found Seq - Paths.get("Hello", zs: _*) // error: immutable.Seq expected, found Array + Paths.get("Hello", zs: _*) } } diff --git a/tests/pos/i4785.scala b/tests/pos/i4785.scala new file mode 100644 index 000000000000..ed12c6a531e3 --- /dev/null +++ b/tests/pos/i4785.scala @@ -0,0 +1,15 @@ +import scala.Predef.Set // unimport Predef.wrapRefArray + +import java.nio.file.Paths + +class i4785 { + def bar(xs: String*) = xs.length + + def test(xs: Seq[String], ys: Array[String]) = { + Paths.get("Hello", xs: _*) + Paths.get("Hello", ys: _*) + + bar(xs: _*) + bar(ys: _*) + } +} From 47d969f60812f3ba20d6f5e8dc435c1ca81be4b9 Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Fri, 7 Dec 2018 18:04:47 +0100 Subject: [PATCH 2/2] Revert 3c4b6bd This is not necessary anymore after #5403 --- .../tools/dotc/transform/ElimRepeated.scala | 36 ++++++++----------- tests/pos/repeatedArgs.scala | 4 +++ 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala index 6c7e452e3d32..4a6185fc0208 100644 --- a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala +++ b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala @@ -71,29 +71,23 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => transformTypeOfTree(tree) override def transformApply(tree: Apply)(implicit ctx: Context): Tree = { - val formals = - ctx.atPhase(thisPhase) { implicit ctx => - tree.fun.tpe.widen.asInstanceOf[MethodType].paramInfos - } - val args1 = tree.args.zipWithConserve(formals) { (arg, formal) => - arg match { - case arg: Typed if isWildcardStarArg(arg) => - val isJavaDefined = tree.fun.symbol.is(JavaDefined) - val tpe = arg.expr.tpe - if (isJavaDefined && tpe.derivesFrom(defn.SeqClass)) - seqToArray(arg.expr, formal.underlyingIfRepeated(isJava = true)) - else if (!isJavaDefined && tpe.derivesFrom(defn.ArrayClass)) - arrayToSeq(arg.expr) - else - arg.expr - case arg => arg - } + val args = tree.args.mapConserve { + case arg: Typed if isWildcardStarArg(arg) => + val isJavaDefined = tree.fun.symbol.is(JavaDefined) + val tpe = arg.expr.tpe + if (isJavaDefined && tpe.derivesFrom(defn.SeqClass)) + seqToArray(arg.expr) + else if (!isJavaDefined && tpe.derivesFrom(defn.ArrayClass)) + arrayToSeq(arg.expr) + else + arg.expr + case arg => arg } - transformTypeOfTree(cpy.Apply(tree)(tree.fun, args1)) + transformTypeOfTree(cpy.Apply(tree)(tree.fun, args)) } - /** Convert sequence argument to Java array of type `pt` */ - private def seqToArray(tree: Tree, pt: Type)(implicit ctx: Context): Tree = tree match { + /** Convert sequence argument to Java array */ + private def seqToArray(tree: Tree)(implicit ctx: Context): Tree = tree match { case SeqLiteral(elems, elemtpt) => JavaSeqLiteral(elems, elemtpt) case _ => @@ -104,8 +98,6 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => .select(nme.seqToArray) .appliedToType(elemType) .appliedTo(tree, Literal(Constant(elemClass.typeRef))) - .ensureConforms(pt) - // Because of phantomclasses, the Java array's type might not conform to the return type } /** Convert Java array argument to Scala Seq */ diff --git a/tests/pos/repeatedArgs.scala b/tests/pos/repeatedArgs.scala index 4bd51e5a15ef..4a57fcf5908e 100644 --- a/tests/pos/repeatedArgs.scala +++ b/tests/pos/repeatedArgs.scala @@ -1,5 +1,6 @@ import scala.collection.{immutable, mutable} import java.nio.file.Paths +import java.util.concurrent.ForkJoinTask class repeatedArgs { def bar(xs: String*): Int = xs.length @@ -19,4 +20,7 @@ class repeatedArgs { val x: collection.Seq[String] = others // val y: immutable.Seq[String] = others // ok in 2.13 } + + def invokeAll[T](tasks: ForkJoinTask[T]*): Unit = ForkJoinTask.invokeAll(tasks: _*) + def invokeAll2(tasks: ForkJoinTask[_]*): Unit = ForkJoinTask.invokeAll(tasks: _*) }