From 1a9c9ebe1fef04bd316aec1173ff617cc3ee0e8b Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 7 Apr 2020 17:16:31 +0200 Subject: [PATCH 1/3] Rename underlyingIfRepeated(isJava) to translateFromRepeated(toArray) The previous name was slightly misleading: we can pass Array sequence arguments to Scala methods and Seq sequence argument to Java methods, so we may want to translate a repeated parameter type to either an Array or a Seq irrespective of what it erases to (we don't right now but we will in the next commit). --- .../tools/dotc/core/TypeApplications.scala | 18 +++++++++--------- .../dotty/tools/dotc/core/TypeErasure.scala | 6 +++--- compiler/src/dotty/tools/dotc/core/Types.scala | 2 +- .../core/unpickleScala2/Scala2Unpickler.scala | 2 +- .../tools/dotc/transform/ElimRepeated.scala | 4 ++-- .../dotc/transform/SyntheticMembers.scala | 2 +- .../tools/dotc/typer/QuotesAndSplices.scala | 2 +- .../src/dotty/tools/dotc/typer/Typer.scala | 4 ++-- 8 files changed, 20 insertions(+), 20 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeApplications.scala b/compiler/src/dotty/tools/dotc/core/TypeApplications.scala index a311463654dd..484692669efb 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeApplications.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeApplications.scala @@ -388,16 +388,16 @@ class TypeApplications(val self: Type) extends AnyVal { else self } - /** If this is repeated parameter type, its underlying Seq type, - * or, if isJava is true, Array type, else the type itself. + /** If this is a repeated parameter `*T`, translate it to either `Seq[T]` or + * `Array[? <: T]` depending on the value of `toArray`, keep other types as + * they are. */ - def underlyingIfRepeated(isJava: Boolean)(implicit ctx: Context): Type = - if (self.isRepeatedParam) { - val seqClass = if (isJava) defn.ArrayClass else defn.SeqClass - // If `isJava` is set, then we want to turn `RepeatedParam[T]` into `Array[? <: T]`, - // since arrays aren't covariant until after erasure. See `tests/pos/i5140`. - translateParameterized(defn.RepeatedParamClass, seqClass, wildcardArg = isJava) - } + def translateFromRepeated(toArray: Boolean)(using Context): Type = + if self.isRepeatedParam then + val seqClass = if (toArray) defn.ArrayClass else defn.SeqClass + // We want `Array[? <: T]` because arrays aren't covariant until after + // erasure. See `tests/pos/i5140`. + translateParameterized(defn.RepeatedParamClass, seqClass, wildcardArg = toArray) else self /** If this is an encoding of a (partially) applied type, return its arguments, diff --git a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala index 8499c897e304..8906a07beb00 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala @@ -155,7 +155,7 @@ object TypeErasure { case etp => etp def sigName(tp: Type, isJava: Boolean)(implicit ctx: Context): TypeName = { - val normTp = tp.underlyingIfRepeated(isJava) + val normTp = tp.translateFromRepeated(toArray = isJava) val erase = erasureFn(isJava, semiEraseVCs = false, isConstructor = false, wildcardOK = true) erase.sigName(normTp)(preErasureCtx) } @@ -448,7 +448,7 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean val tycon = tp.tycon if (tycon.isRef(defn.ArrayClass)) eraseArray(tp) else if (tycon.isRef(defn.PairClass)) erasePair(tp) - else if (tp.isRepeatedParam) apply(tp.underlyingIfRepeated(isJava)) + else if (tp.isRepeatedParam) apply(tp.translateFromRepeated(toArray = isJava)) else apply(tp.translucentSuperType) case _: TermRef | _: ThisType => this(tp.widen) @@ -540,7 +540,7 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean // See doc comment for ElimByName for speculation how we could improve this. else MethodType(Nil, Nil, - eraseResult(sym.info.finalResultType.underlyingIfRepeated(isJava))) + eraseResult(sym.info.finalResultType.translateFromRepeated(toArray = isJava))) case tp1: PolyType => eraseResult(tp1.resultType) match case rt: MethodType => rt diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index a401645d8f6b..b3629dc2386b 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -1607,7 +1607,7 @@ object Types { case res => res } val funType = defn.FunctionOf( - formals1 mapConserve (_.underlyingIfRepeated(mt.isJavaMethod)), + formals1 mapConserve (_.translateFromRepeated(toArray = mt.isJavaMethod)), result1, isContextual, isErased) if (mt.isResultDependent) RefinedType(funType, nme.apply, mt) else funType diff --git a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala index c374bfc0acb4..67d626270a7e 100644 --- a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala @@ -593,7 +593,7 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas if (tag == ALIASsym) TypeAlias(tp1) else if (denot.isType) checkNonCyclic(denot.symbol, tp1, reportErrors = false) // we need the checkNonCyclic call to insert LazyRefs for F-bounded cycles - else if (!denot.is(Param)) tp1.underlyingIfRepeated(isJava = false) + else if (!denot.is(Param)) tp1.translateFromRepeated(toArray = false) else tp1 if (denot.isConstructor) addConstructorTypeParams(denot) if (atEnd) diff --git a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala index dcb680e77b7d..e4c75b04cc7e 100644 --- a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala +++ b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala @@ -52,7 +52,7 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => val resultType1 = elimRepeated(resultType) val paramTypes1 = if (paramTypes.nonEmpty && paramTypes.last.isRepeatedParam) { - val last = paramTypes.last.underlyingIfRepeated(tp.isJavaMethod) + val last = paramTypes.last.translateFromRepeated(toArray = tp.isJavaMethod) paramTypes.init :+ last } else paramTypes @@ -159,7 +159,7 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => tp.derivedLambdaType(tp.paramNames, tp.paramInfos, toJavaVarArgs(tp.resultType)) case tp: MethodType => val inits :+ last = tp.paramInfos - val last1 = last.underlyingIfRepeated(isJava = true) + val last1 = last.translateFromRepeated(toArray = true) tp.derivedLambdaType(tp.paramNames, inits :+ last1, tp.resultType) } } diff --git a/compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala b/compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala index e25ca9a160d2..acb731dfa293 100644 --- a/compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala +++ b/compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala @@ -420,7 +420,7 @@ class SyntheticMembers(thisPhase: DenotTransformer) { for ((formal, idx) <- methTpe.paramInfos.zipWithIndex) yield { val elem = param.select(defn.Product_productElement).appliedTo(Literal(Constant(idx))) - .ensureConforms(formal.underlyingIfRepeated(isJava = false)) + .ensureConforms(formal.translateFromRepeated(toArray = false)) if (formal.isRepeatedParam) ctx.typer.seqToRepeated(elem) else elem } New(classRef, elems) diff --git a/compiler/src/dotty/tools/dotc/typer/QuotesAndSplices.scala b/compiler/src/dotty/tools/dotc/typer/QuotesAndSplices.scala index dab14aa2a90f..27452c835c30 100644 --- a/compiler/src/dotty/tools/dotc/typer/QuotesAndSplices.scala +++ b/compiler/src/dotty/tools/dotc/typer/QuotesAndSplices.scala @@ -207,7 +207,7 @@ trait QuotesAndSplices { try ref(defn.InternalQuoted_patternHole.termRef).appliedToType(tree.tpe).withSpan(tree.span) finally { val patType = pat.tpe.widen - val patType1 = patType.underlyingIfRepeated(isJava = false) + val patType1 = patType.translateFromRepeated(toArray = false) val pat1 = if (patType eq patType1) pat else pat.withType(patType1) patBuf += pat1 } diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 78093dbfcee7..bacfd6fdec98 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -729,7 +729,7 @@ class Typer extends Namer if (untpd.isWildcardStarArg(tree)) { def typedWildcardStarArgExpr = { val ptArg = - if (ctx.mode.is(Mode.QuotedPattern)) pt.underlyingIfRepeated(isJava = false) + if (ctx.mode.is(Mode.QuotedPattern)) pt.translateFromRepeated(toArray = false) else WildcardType val tpdExpr = typedExpr(tree.expr, ptArg) tpdExpr.tpe.widenDealias match { @@ -1158,7 +1158,7 @@ class Typer extends Namer if (!param.tpt.isEmpty) param else cpy.ValDef(param)( tpt = untpd.TypeTree( - inferredParamType(param, protoFormal(i)).underlyingIfRepeated(isJava = false))) + inferredParamType(param, protoFormal(i)).translateFromRepeated(toArray = false))) desugar.makeClosure(inferredParams, fnBody, resultTpt, isContextual) } typed(desugared, pt) From 2c5a02902f5f822bf1a26800f1ebdbf1d9a8312a Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Fri, 3 Apr 2020 15:15:13 +0200 Subject: [PATCH 2/3] Take expected type into account when typing a sequence argument Given an expected type `*T`, we allow a sequence argument `xs: _*` to be either a `Seq[T]` or an `Array[_ <: T]`, irrespective of whether the method we're calling is a Java or Scala method. So far we typed sequence arguments without an expected type, meaning that adaptation did not take place. But thanks to #8635, we can type them with an expected type of `Seq[T] | Array[_ <: T]` and type inference works out. This is what this commit does. --- .../community-projects/xml-interpolator | 2 +- .../tools/dotc/core/TypeApplications.scala | 14 ++++++---- .../src/dotty/tools/dotc/typer/Typer.scala | 17 +++++------ tests/pos/case-signature.scala | 5 ++++ tests/pos/sequence-argument/B_2.scala | 28 +++++++++++++++++++ tests/pos/sequence-argument/Java_2.java | 5 ++++ tests/pos/sequence-argument/XY_1.scala | 2 ++ 7 files changed, 59 insertions(+), 14 deletions(-) create mode 100644 tests/pos/case-signature.scala create mode 100644 tests/pos/sequence-argument/B_2.scala create mode 100644 tests/pos/sequence-argument/Java_2.java create mode 100644 tests/pos/sequence-argument/XY_1.scala diff --git a/community-build/community-projects/xml-interpolator b/community-build/community-projects/xml-interpolator index 878c40693839..73ad802d216a 160000 --- a/community-build/community-projects/xml-interpolator +++ b/community-build/community-projects/xml-interpolator @@ -1 +1 @@ -Subproject commit 878c40693839ae51ae35c54ad751f184053091dd +Subproject commit 73ad802d216a7377ab6dfe62febdc02a7da95261 diff --git a/compiler/src/dotty/tools/dotc/core/TypeApplications.scala b/compiler/src/dotty/tools/dotc/core/TypeApplications.scala index 484692669efb..2e3316361068 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeApplications.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeApplications.scala @@ -389,12 +389,16 @@ class TypeApplications(val self: Type) extends AnyVal { } /** If this is a repeated parameter `*T`, translate it to either `Seq[T]` or - * `Array[? <: T]` depending on the value of `toArray`, keep other types as - * they are. + * `Array[? <: T]` depending on the value of `toArray`. + * Additionally, if `translateWildcard` is true, a wildcard type + * will be translated to `*`. + * Other types are kept as-is. */ - def translateFromRepeated(toArray: Boolean)(using Context): Type = - if self.isRepeatedParam then - val seqClass = if (toArray) defn.ArrayClass else defn.SeqClass + def translateFromRepeated(toArray: Boolean, translateWildcard: Boolean = false)(using Context): Type = + val seqClass = if (toArray) defn.ArrayClass else defn.SeqClass + if translateWildcard && self.isInstanceOf[WildcardType] then + seqClass.typeRef.appliedTo(WildcardType) + else if self.isRepeatedParam then // We want `Array[? <: T]` because arrays aren't covariant until after // erasure. See `tests/pos/i5140`. translateParameterized(defn.RepeatedParamClass, seqClass, wildcardArg = toArray) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index bacfd6fdec98..1dcb777c01d5 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -728,19 +728,20 @@ class Typer extends Namer if (untpd.isWildcardStarArg(tree)) { def typedWildcardStarArgExpr = { + // A sequence argument `xs: _*` can be either a `Seq[T]` or an `Array[_ <: T]`, + // irrespective of whether the method we're calling is a Java or Scala method, + // so the expected type is the union `Seq[T] | Array[_ <: T]`. val ptArg = - if (ctx.mode.is(Mode.QuotedPattern)) pt.translateFromRepeated(toArray = false) - else WildcardType + // FIXME(#8680): Quoted patterns do not support Array repeated arguments + if (ctx.mode.is(Mode.QuotedPattern)) pt.translateFromRepeated(toArray = false, translateWildcard = true) + else pt.translateFromRepeated(toArray = false, translateWildcard = true) | + pt.translateFromRepeated(toArray = true, translateWildcard = true) val tpdExpr = typedExpr(tree.expr, ptArg) tpdExpr.tpe.widenDealias match { case defn.ArrayOf(_) => - val starType = defn.ArrayType.appliedTo(WildcardType) - val exprAdapted = adapt(tpdExpr, starType) - arrayToRepeated(exprAdapted) + arrayToRepeated(tpdExpr) case _ => - val starType = defn.SeqType.appliedTo(defn.AnyType) - val exprAdapted = adapt(tpdExpr, starType) - seqToRepeated(exprAdapted) + seqToRepeated(tpdExpr) } } cases( diff --git a/tests/pos/case-signature.scala b/tests/pos/case-signature.scala new file mode 100644 index 000000000000..0a814b778d17 --- /dev/null +++ b/tests/pos/case-signature.scala @@ -0,0 +1,5 @@ +// If `translateFromRepeated` translated wildcards by default, the following +// would break because of the use of wildcards in signatures. +case class Benchmark[A](params: List[A], + sqlInsert: (benchId: Long, params: A, session: Int) => Unit, + fun: List[A]) diff --git a/tests/pos/sequence-argument/B_2.scala b/tests/pos/sequence-argument/B_2.scala new file mode 100644 index 000000000000..97c1817dade1 --- /dev/null +++ b/tests/pos/sequence-argument/B_2.scala @@ -0,0 +1,28 @@ +import scala.reflect.ClassTag +import scala.language.implicitConversions + +object B { + def doubleSeq[T](x: T): Seq[T] = Seq(x, x) + def doubleArray[T: ClassTag](x: T): Array[T] = Array(x, x) + + def box(args: Integer*): Unit = {} + def widen(args: Long*): Unit = {} + def conv(args: Y*): Unit = {} + + box(doubleSeq(1): _*) + box(doubleArray(1): _*) + Java_2.box(doubleSeq(1): _*) + Java_2.box(doubleArray(1): _*) + + widen(doubleSeq(1): _*) + widen(doubleArray(1): _*) + Java_2.widen(doubleSeq(1): _*) + Java_2.widen(doubleArray(1): _*) + + implicit def xToY(x: X): Y = new Y + val x: X = new X + conv(doubleSeq(x): _*) + conv(doubleArray(x): _*) + Java_2.conv(doubleSeq(x): _*) + Java_2.conv(doubleArray(x): _*) +} diff --git a/tests/pos/sequence-argument/Java_2.java b/tests/pos/sequence-argument/Java_2.java new file mode 100644 index 000000000000..5d8ac083bcf9 --- /dev/null +++ b/tests/pos/sequence-argument/Java_2.java @@ -0,0 +1,5 @@ +public class Java_2 { + public static void box(Integer ...args) {} + public static void widen(Long... args) {} + public static void conv(Y... args) {} +} diff --git a/tests/pos/sequence-argument/XY_1.scala b/tests/pos/sequence-argument/XY_1.scala new file mode 100644 index 000000000000..947da159ab3c --- /dev/null +++ b/tests/pos/sequence-argument/XY_1.scala @@ -0,0 +1,2 @@ +class X +class Y From 03d5d9b93274dfe66b454c840e7317e81380b240 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 7 Apr 2020 18:42:00 +0200 Subject: [PATCH 3/3] Keep span and attachments when typing a sequence argument I don't know of a case where this matters currently but it's more correct. --- .../src/dotty/tools/dotc/core/TypeApplications.scala | 6 +++++- compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Typer.scala | 10 ++++------ 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeApplications.scala b/compiler/src/dotty/tools/dotc/core/TypeApplications.scala index 2e3316361068..9ea66b9e4f07 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeApplications.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeApplications.scala @@ -378,7 +378,7 @@ class TypeApplications(val self: Type) extends AnyVal { self.derivedExprType(tp.translateParameterized(from, to)) case _ => if (self.derivesFrom(from)) { - def elemType(tp: Type): Type = tp match + def elemType(tp: Type): Type = tp.widenDealias match case tp: AndOrType => tp.derivedAndOrType(elemType(tp.tp1), elemType(tp.tp2)) case _ => tp.baseType(from).argInfos.head val arg = elemType(self) @@ -404,6 +404,10 @@ class TypeApplications(val self: Type) extends AnyVal { translateParameterized(defn.RepeatedParamClass, seqClass, wildcardArg = toArray) else self + /** Translate a `From[T]` into a `*T`. */ + def translateToRepeated(from: ClassSymbol)(using Context): Type = + translateParameterized(from, defn.RepeatedParamClass) + /** If this is an encoding of a (partially) applied type, return its arguments, * otherwise return Nil. * Existential types in arguments are returned as TypeBounds instances. diff --git a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala index da1ed86fdbee..35eb5b534c35 100644 --- a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -166,7 +166,7 @@ trait TypeAssigner { else sym.info private def toRepeated(tree: Tree, from: ClassSymbol)(using Context): Tree = - Typed(tree, TypeTree(tree.tpe.widen.translateParameterized(from, defn.RepeatedParamClass))) + Typed(tree, TypeTree(tree.tpe.widen.translateToRepeated(from))) def seqToRepeated(tree: Tree)(using Context): Tree = toRepeated(tree, defn.SeqClass) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 1dcb777c01d5..159b6ce90d14 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -737,12 +737,10 @@ class Typer extends Namer else pt.translateFromRepeated(toArray = false, translateWildcard = true) | pt.translateFromRepeated(toArray = true, translateWildcard = true) val tpdExpr = typedExpr(tree.expr, ptArg) - tpdExpr.tpe.widenDealias match { - case defn.ArrayOf(_) => - arrayToRepeated(tpdExpr) - case _ => - seqToRepeated(tpdExpr) - } + val expr1 = typedExpr(tree.expr, ptArg) + val fromCls = if expr1.tpe.derivesFrom(defn.ArrayClass) then defn.ArrayClass else defn.SeqClass + val tpt1 = TypeTree(expr1.tpe.widen.translateToRepeated(fromCls)).withSpan(tree.tpt.span) + assignType(cpy.Typed(tree)(expr1, tpt1), tpt1) } cases( ifPat = ascription(TypeTree(defn.RepeatedParamType.appliedTo(pt)), isWildcard = true),