From f50c1f7c81cb796ffc27c8205fba57c6f1821e78 Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Wed, 18 Jul 2018 17:06:11 +0200 Subject: [PATCH 1/4] Resolve SeqType based on the standard library on the classpath - 2.12: scala.collection.Seq - 2.13: scala.collection.immutable.Seq --- compiler/src/dotty/tools/dotc/ast/tpd.scala | 21 ++++++++- .../tools/dotc/config/ScalaSettings.scala | 1 + .../dotty/tools/dotc/core/Definitions.scala | 28 +++++++----- .../tools/dotc/transform/ElimRepeated.scala | 4 +- .../tools/dotc/transform/SeqLiterals.scala | 10 +---- .../dotty/tools/dotc/transform/TreeGen.scala | 26 ----------- .../dotty/tools/dotc/typer/Implicits.scala | 2 +- tests/neg/repeatedArgs213.scala | 43 +++++++++++++++++++ tests/pos/repeatedArgs.scala | 23 +++++++++- tests/pos/repeatedArgs213.scala | 42 ++++++++++++++++++ 10 files changed, 148 insertions(+), 52 deletions(-) delete mode 100644 compiler/src/dotty/tools/dotc/transform/TreeGen.scala create mode 100644 tests/neg/repeatedArgs213.scala create mode 100644 tests/pos/repeatedArgs213.scala diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index b9a6edcc6bc3..440266d32f3e 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -5,6 +5,7 @@ package ast import dotty.tools.dotc.transform.{ExplicitOuter, Erasure} import dotty.tools.dotc.typer.ProtoTypes.FunProtoTyped import transform.SymUtils._ +import transform.TypeUtils._ import core._ import util.Positions._, Types._, Contexts._, Constants._, Names._, Flags._, NameOps._ import SymDenotations._, Symbols._, StdNames._, Annotations._, Trees._, Symbols._ @@ -377,7 +378,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { } /** A tree representing a `newXYZArray` operation of the right - * kind for the given element type in `typeArg`. No type arguments or + * kind for the given element type in `elemTpe`. No type arguments or * `length` arguments are given. */ def newArray(elemTpe: Type, returnTpe: Type, pos: Position, dims: JavaSeqLiteral)(implicit ctx: Context): Tree = { @@ -392,6 +393,24 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { newArr.appliedToArgs(clsOf(elemTpe) :: clsOf(returnTpe) :: dims :: Nil).withPos(pos) } + /** The wrapped array method name for an array of type elemtp */ + def wrapArrayMethodName(elemtp: Type)(implicit ctx: Context): TermName = { + val elemCls = elemtp.classSymbol + if (elemCls.isPrimitiveValueClass) nme.wrapXArray(elemCls.name) + else if (elemCls.derivesFrom(defn.ObjectClass) && !elemCls.isNotRuntimeClass) nme.wrapRefArray + else nme.genericWrapArray + } + + /** A tree representing a `wrapXYZArray(tree)` operation of the right + * kind for the given element type in `elemTpe`. + */ + def wrapArray(tree: Tree, elemtp: Type)(implicit ctx: Context): Tree = { + ref(defn.getWrapVarargsArrayModule) + .select(wrapArrayMethodName(elemtp)) + .appliedToTypes(if (elemtp.isPrimitiveValueType) Nil else elemtp :: Nil) + .appliedTo(tree) + } + // ------ Creating typed equivalents of trees that exist only in untyped form ------- /** new C(args), calling the primary constructor of C */ diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 94d1b689c9e0..f31a1935f767 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -124,6 +124,7 @@ class ScalaSettings extends Settings.SettingGroup { val YcheckAllPatmat = BooleanSetting("-Ycheck-all-patmat", "Check exhaustivity and redundancy of all pattern matching (used for testing the algorithm)") val YretainTrees = BooleanSetting("-Yretain-trees", "Retain trees for top-level classes, accessible from ClassSymbol#tree") val YshowTreeIds = BooleanSetting("-Yshow-tree-ids", "Uniquely tag all tree nodes in debugging output.") + val YimmutableSeq = BooleanSetting("-Yimmutable-seq", "Use collection.immutable.Seq as the default Seq type") val YprofileEnabled = BooleanSetting("-Yprofile-enabled", "Enable profiling.") val YprofileDestination = StringSetting("-Yprofile-destination", "file", "where to send profiling output - specify a file, default is to the console.", "") diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 18f46407ddd8..30ecfea009d2 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -9,7 +9,6 @@ import scala.collection.{ mutable, immutable } import PartialFunction._ import collection.mutable import util.common.alwaysZero -import dotty.tools.dotc.transform.TreeGen object Definitions { @@ -345,11 +344,6 @@ class Definitions { def Predef_classOf(implicit ctx: Context) = Predef_classOfR.symbol lazy val Predef_undefinedR = ScalaPredefModule.requiredMethodRef("???") def Predef_undefined(implicit ctx: Context) = Predef_undefinedR.symbol - // The set of all wrap{X, Ref}Array methods, where X is a value type - val Predef_wrapArray = new PerRun[collection.Set[Symbol]]({ implicit ctx => - val methodNames = ScalaValueTypes.map(TreeGen.wrapArrayMethodName) + nme.wrapRefArray - methodNames.map(ScalaPredefModule.requiredMethodRef(_).symbol) - }) lazy val ScalaRuntimeModuleRef = ctx.requiredModuleRef("scala.runtime.ScalaRunTime") def ScalaRuntimeModule(implicit ctx: Context) = ScalaRuntimeModuleRef.symbol @@ -390,6 +384,17 @@ class Definitions { def newGenericArrayMethod(implicit ctx: Context) = DottyArraysModule.requiredMethod("newGenericArray") def newArrayMethod(implicit ctx: Context) = DottyArraysModule.requiredMethod("newArray") + // TODO: Remove once we drop support for 2.12 standard library + private[this] lazy val isNewCollections = ctx.base.staticRef("scala.collection.IterableOnce".toTypeName).exists + + def getWrapVarargsArrayModule = if (isNewCollections) ScalaRuntimeModule else ScalaPredefModule + + // The set of all wrap{X, Ref}Array methods, where X is a value type + val WrapArrayMethods = new PerRun[collection.Set[Symbol]]({ implicit ctx => + val methodNames = ScalaValueTypes.map(ast.tpd.wrapArrayMethodName) + nme.wrapRefArray + methodNames.map(getWrapVarargsArrayModule.requiredMethodRef(_).symbol) + }) + lazy val NilModuleRef = ctx.requiredModuleRef("scala.collection.immutable.Nil") def NilModule(implicit ctx: Context) = NilModuleRef.symbol @@ -401,7 +406,9 @@ class Definitions { List(AnyClass.typeRef), EmptyScope) lazy val SingletonType: TypeRef = SingletonClass.typeRef - lazy val SeqType: TypeRef = ctx.requiredClassRef("scala.collection.Seq") + lazy val SeqType: TypeRef = + if (isNewCollections) ctx.requiredClassRef("scala.collection.immutable.Seq") + else ctx.requiredClassRef("scala.collection.Seq") def SeqClass(implicit ctx: Context) = SeqType.symbol.asClass lazy val Seq_applyR = SeqClass.requiredMethodRef(nme.apply) def Seq_apply(implicit ctx: Context) = Seq_applyR.symbol @@ -1210,12 +1217,11 @@ class Definitions { lazy val reservedScalaClassNames: Set[Name] = syntheticScalaClasses.map(_.name).toSet - private[this] var _isInitialized = false - private def isInitialized = _isInitialized + private[this] var isInitialized = false def init()(implicit ctx: Context) = { this.ctx = ctx - if (!_isInitialized) { + if (!isInitialized) { // Enter all symbols from the scalaShadowing package in the scala package for (m <- ScalaShadowingPackageClass.info.decls) ScalaPackageClass.enter(m) @@ -1230,7 +1236,7 @@ class Definitions { // force initialization of every symbol that is synthesized or hijacked by the compiler val forced = syntheticCoreClasses ++ syntheticCoreMethods ++ ScalaValueClasses() - _isInitialized = true + isInitialized = true } } diff --git a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala index 23b6dfb4dd5e..c94631e78d30 100644 --- a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala +++ b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala @@ -98,7 +98,7 @@ 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.Predef_wrapArray().contains(fun.symbol) => // rewrite a call to `wrapXArray(arr)` to `arr` + 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 @@ -143,7 +143,7 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => val elemtp = varArgRef.tpe.widen.argTypes.head ref(original.termRef) .appliedToTypes(trefs) - .appliedToArgs(vrefs :+ TreeGen.wrapArray(varArgRef, elemtp)) + .appliedToArgs(vrefs :+ tpd.wrapArray(varArgRef, elemtp)) .appliedToArgss(vrefss1) }) diff --git a/compiler/src/dotty/tools/dotc/transform/SeqLiterals.scala b/compiler/src/dotty/tools/dotc/transform/SeqLiterals.scala index 756e4f045cb6..2e17a211a04d 100644 --- a/compiler/src/dotty/tools/dotc/transform/SeqLiterals.scala +++ b/compiler/src/dotty/tools/dotc/transform/SeqLiterals.scala @@ -35,14 +35,6 @@ class SeqLiterals extends MiniPhase { val arr = JavaSeqLiteral(tree.elems, tree.elemtpt) //println(i"trans seq $tree, arr = $arr: ${arr.tpe} ${arr.tpe.elemType}") val elemtp = tree.elemtpt.tpe - val elemCls = elemtp.classSymbol - val (wrapMethStr, targs) = - if (elemCls.isPrimitiveValueClass) (s"wrap${elemCls.name}Array", Nil) - else if (elemtp derivesFrom defn.ObjectClass) ("wrapRefArray", elemtp :: Nil) - else ("genericWrapArray", elemtp :: Nil) - ref(defn.ScalaPredefModule) - .select(wrapMethStr.toTermName) - .appliedToTypes(targs) - .appliedTo(arr) + wrapArray(arr, elemtp) } } diff --git a/compiler/src/dotty/tools/dotc/transform/TreeGen.scala b/compiler/src/dotty/tools/dotc/transform/TreeGen.scala deleted file mode 100644 index a6dfdebc3cf4..000000000000 --- a/compiler/src/dotty/tools/dotc/transform/TreeGen.scala +++ /dev/null @@ -1,26 +0,0 @@ -package dotty.tools.dotc -package transform - -import core._ -import Symbols._, Contexts._, Types._, Names._, StdNames._ -import ast._ -import Trees._ -import TypeUtils._ - -object TreeGen { - - import tpd._ - - def wrapArrayMethodName(elemtp: Type)(implicit ctx: Context): TermName = { - val elemCls = elemtp.classSymbol - if (elemCls.isPrimitiveValueClass) nme.wrapXArray(elemCls.name) - else if (elemCls.derivesFrom(defn.ObjectClass) && !elemCls.isNotRuntimeClass) nme.wrapRefArray - else nme.genericWrapArray - } - - def wrapArray(tree: Tree, elemtp: Type)(implicit ctx: Context): Tree = - ref(defn.ScalaPredefModule) - .select(wrapArrayMethodName(elemtp)) - .appliedToTypes(if (elemtp.isPrimitiveValueType) Nil else elemtp :: Nil) - .appliedTo(tree) -} diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 848d928d7cf5..4ba82bc897b9 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -71,7 +71,7 @@ object Implicits { /** The implicit references */ def refs: List[ImplicitRef] - private var SingletonClass: ClassSymbol = null + private[this] var SingletonClass: ClassSymbol = null /** Widen type so that it is neither a singleton type nor a type that inherits from scala.Singleton. */ private def widenSingleton(tp: Type)(implicit ctx: Context): Type = { diff --git a/tests/neg/repeatedArgs213.scala b/tests/neg/repeatedArgs213.scala new file mode 100644 index 000000000000..e650a371ecc3 --- /dev/null +++ b/tests/neg/repeatedArgs213.scala @@ -0,0 +1,43 @@ +import scala.collection.{immutable, mutable} +import java.nio.file.Paths + +// Code below is to trick the compiler into thinking that we are +// compiling with the 2.13 standard library on the classpath. +package scala.collection { + class IterableOnce +} + +package scala.runtime { + object ScalaRunTime { + abstract class ArraySeq[+A] extends immutable.Seq[A] + + def genericWrapArray[T](xs: Array[T]): ArraySeq[T] = ??? + def wrapRefArray[T <: AnyRef](xs: Array[T]): ArraySeq[T] = ??? + def wrapIntArray(xs: Array[Int]): ArraySeq[Int] = ??? + def wrapDoubleArray(xs: Array[Double]): ArraySeq[Double] = ??? + def wrapLongArray(xs: Array[Long]): ArraySeq[Long] = ??? + def wrapFloatArray(xs: Array[Float]): ArraySeq[Float] = ??? + def wrapCharArray(xs: Array[Char]): ArraySeq[Char] = ??? + def wrapByteArray(xs: Array[Byte]): ArraySeq[Byte] = ??? + def wrapShortArray(xs: Array[Short]): ArraySeq[Short] = ??? + def wrapBooleanArray(xs: Array[Boolean]): ArraySeq[Boolean] = ??? + def wrapUnitArray(xs: Array[Unit]): ArraySeq[Unit] = ??? + } +} + +// Start of Test +class repeatedArgs { + def bar(xs: String*): Int = xs.length + + def test(xs: immutable.Seq[String], ys: collection.Seq[String], zs: Array[String]): Unit = { + bar("a", "b", "c") + bar(xs: _*) + bar(ys: _*) // error: immutable.Seq expected, found Seq + bar(zs: _*) // error: immutable.Seq expected, found Array + + 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 + } +} diff --git a/tests/pos/repeatedArgs.scala b/tests/pos/repeatedArgs.scala index f9abb4aee70e..4bd51e5a15ef 100644 --- a/tests/pos/repeatedArgs.scala +++ b/tests/pos/repeatedArgs.scala @@ -1,3 +1,22 @@ -object testRepeated { - def foo = java.lang.System.out.format("%4$2s %3$2s %2$2s %1$2s", "a", "b", "c", "d") +import scala.collection.{immutable, mutable} +import java.nio.file.Paths + +class repeatedArgs { + def bar(xs: String*): Int = xs.length + + def test(xs: immutable.Seq[String], ys: collection.Seq[String], zs: Array[String]): Unit = { + bar("a", "b", "c") + bar(xs: _*) + bar(ys: _*) // error in 2.13 + bar(zs: _*) + + Paths.get("Hello", "World") + Paths.get("Hello", xs: _*) + Paths.get("Hello", ys: _*) // error in 2.13 + Paths.get("Hello", zs: _*) + + val List(_, others: _*) = xs.toList // toList should not be needed, see #4790 + val x: collection.Seq[String] = others + // val y: immutable.Seq[String] = others // ok in 2.13 + } } diff --git a/tests/pos/repeatedArgs213.scala b/tests/pos/repeatedArgs213.scala new file mode 100644 index 000000000000..b6b52513ff69 --- /dev/null +++ b/tests/pos/repeatedArgs213.scala @@ -0,0 +1,42 @@ +import scala.collection.{immutable, mutable} +import java.nio.file.Paths + +// Code below is to trick the compiler into thinking that we are +// compiling with the 2.13 standard library on the classpath. +package scala.collection { + class IterableOnce +} + +package scala.runtime { + object ScalaRunTime { + abstract class ArraySeq[+A] extends immutable.Seq[A] + + def genericWrapArray[T](xs: Array[T]): ArraySeq[T] = ??? + def wrapRefArray[T <: AnyRef](xs: Array[T]): ArraySeq[T] = ??? + def wrapIntArray(xs: Array[Int]): ArraySeq[Int] = ??? + def wrapDoubleArray(xs: Array[Double]): ArraySeq[Double] = ??? + def wrapLongArray(xs: Array[Long]): ArraySeq[Long] = ??? + def wrapFloatArray(xs: Array[Float]): ArraySeq[Float] = ??? + def wrapCharArray(xs: Array[Char]): ArraySeq[Char] = ??? + def wrapByteArray(xs: Array[Byte]): ArraySeq[Byte] = ??? + def wrapShortArray(xs: Array[Short]): ArraySeq[Short] = ??? + def wrapBooleanArray(xs: Array[Boolean]): ArraySeq[Boolean] = ??? + def wrapUnitArray(xs: Array[Unit]): ArraySeq[Unit] = ??? + } +} + +// Start of Test +class repeatedArgs { + def bar(xs: String*): Int = xs.length + + def test(xs: immutable.Seq[String]): Unit = { + bar("a", "b", "c") + bar(xs: _*) + + Paths.get("Hello", "World") + Paths.get("Hello", xs: _*) + + val List(_, others: _*) = xs.toList // toList should not be needed, see #4790 + val x: immutable.Seq[String] = others + } +} From 2c9e845a72a9bcb95063e83ba5d28882a2947233 Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Wed, 18 Jul 2018 17:14:33 +0200 Subject: [PATCH 2/4] Add test that exercises a cycle reference when playing with SeqType --- tests/pos/seqtype-cycle/Test1.scala | 3 +++ tests/pos/seqtype-cycle/Test2.scala | 8 ++++++++ tests/pos/seqtype-cycle/Test3.scala | 12 ++++++++++++ 3 files changed, 23 insertions(+) create mode 100644 tests/pos/seqtype-cycle/Test1.scala create mode 100644 tests/pos/seqtype-cycle/Test2.scala create mode 100644 tests/pos/seqtype-cycle/Test3.scala diff --git a/tests/pos/seqtype-cycle/Test1.scala b/tests/pos/seqtype-cycle/Test1.scala new file mode 100644 index 000000000000..ee8d10d7c36a --- /dev/null +++ b/tests/pos/seqtype-cycle/Test1.scala @@ -0,0 +1,3 @@ +class Test { + def bar = Array(1) // call to Array(_: Repeated) +} diff --git a/tests/pos/seqtype-cycle/Test2.scala b/tests/pos/seqtype-cycle/Test2.scala new file mode 100644 index 000000000000..a9e1fc7ee214 --- /dev/null +++ b/tests/pos/seqtype-cycle/Test2.scala @@ -0,0 +1,8 @@ +package object scala { + // needed for some reasons + type Throwable = java.lang.Throwable + type IndexOutOfBoundsException = java.lang.IndexOutOfBoundsException + + type Seq[+A] = scala.collection.Seq[A] + val Seq = scala.collection.Seq +} diff --git a/tests/pos/seqtype-cycle/Test3.scala b/tests/pos/seqtype-cycle/Test3.scala new file mode 100644 index 000000000000..f137ded9ffa0 --- /dev/null +++ b/tests/pos/seqtype-cycle/Test3.scala @@ -0,0 +1,12 @@ +package scala + +class specialized(types: Int*) extends scala.annotation.StaticAnnotation + +package collection { + class Foo[@specialized(1, 2) A] + + trait Seq[A] extends Foo[A] + object Seq extends SeqFactory[Seq] + + trait SeqFactory[CC[X] <: Seq[X]] +} From f6a67143cd5f85cf56c9a12b593f3826db3cf018 Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Wed, 18 Jul 2018 17:59:50 +0200 Subject: [PATCH 3/4] Add failing test to FromTasty blacklist Compiling "core" classes from TASTY doesn't seem to be possible at this time. --- compiler/test/dotty/tools/dotc/FromTastyTests.scala | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/test/dotty/tools/dotc/FromTastyTests.scala b/compiler/test/dotty/tools/dotc/FromTastyTests.scala index 6c992a0be945..f4817fee9876 100644 --- a/compiler/test/dotty/tools/dotc/FromTastyTests.scala +++ b/compiler/test/dotty/tools/dotc/FromTastyTests.scala @@ -39,6 +39,9 @@ class FromTastyTests extends ParallelTesting { // Fails on MacOS "annot-bootstrap.scala", + + // ScalaRunTime cannot be unpickled because it is already loaded + "repeatedArgs213.scala", ), recompilationBlacklist = Set( ) From c5b3bfb1bf7cb2bed4eb97b11368afaeba473fe5 Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Wed, 18 Jul 2018 18:27:24 +0200 Subject: [PATCH 4/4] Remove dead code --- compiler/src/dotty/tools/dotc/config/ScalaSettings.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index f31a1935f767..94d1b689c9e0 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -124,7 +124,6 @@ class ScalaSettings extends Settings.SettingGroup { val YcheckAllPatmat = BooleanSetting("-Ycheck-all-patmat", "Check exhaustivity and redundancy of all pattern matching (used for testing the algorithm)") val YretainTrees = BooleanSetting("-Yretain-trees", "Retain trees for top-level classes, accessible from ClassSymbol#tree") val YshowTreeIds = BooleanSetting("-Yshow-tree-ids", "Uniquely tag all tree nodes in debugging output.") - val YimmutableSeq = BooleanSetting("-Yimmutable-seq", "Use collection.immutable.Seq as the default Seq type") val YprofileEnabled = BooleanSetting("-Yprofile-enabled", "Enable profiling.") val YprofileDestination = StringSetting("-Yprofile-destination", "file", "where to send profiling output - specify a file, default is to the console.", "")