Skip to content

Arrays and Repeated arguments in 2.13 and future Dotty version #4785

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
allanrenucci opened this issue Jul 11, 2018 · 5 comments
Closed

Arrays and Repeated arguments in 2.13 and future Dotty version #4785

allanrenucci opened this issue Jul 11, 2018 · 5 comments

Comments

@allanrenucci
Copy link
Contributor

Starting with Scala 2.13 the default Seq type is immutable.Seq. This change apply to repeated arguments:

scala> def foo(x: Int*) = 1
foo: (x: Int*)Int

scala> foo(immutable.Seq(1, 2): _*)
res0: Int = 1

scala> foo(mutable.Seq(1, 2): _*)
                      ^
       error: type mismatch;
        found   : Seq[Int] (in scala.collection.mutable)
        required: Seq[Int] (in scala.collection.immutable)

scala> foo(Array(1, 2): _*)
res2: Int = 1

However, it is still possible to pass an Array where repeated arguments are expected. This is a requirement for java interoperability.

In Dotty, given foo: _*, foo must be a subtype of Seq. If foo is an array, then an implicit conversion is applied (i.e. Predef.wrapXXXArray). We don't need to special case arrays in typer but latter (in ElimRepeated) we have to revert the conversion if the array is used as a repeated arguments of a java defined method. E.g.

// xs: Array[String]

java.nio.file.Paths.get("foo", xs: _*)

// >>>>>> Typer
java.nio.file.Paths.get("foo", Predef.wrapRefArray(xs): String*)

// >>>>>> ElimRepeated
java.nio.file.Paths.get("foo", xs)

The current implicit conversions defined in Predef returns a mutable.Seq.

How do we solve this problem in the future when Dotty switch its default Seq type to immutable.Seq instead of collection.Seq? Here are my thoughts, either:

  1. Introduce new implicit conversions in DottyPredef (or Predef?) from Array to immutable.Seq.
  2. Change typing rules for repeated arguments. In expression position:
    foo <:< Array[A] or foo <:< immutable.Seq[A]
    ---------------------------------------------
    foo: _* ⊢ Repeated[A]
    

I think scalac does 2). @adriaanm, any thoughts on this?

@julienrf
Copy link
Contributor

julienrf commented Jul 11, 2018

I don’t think scalac has special cased the typing rule for splicing arrays as multiple parameters. If you write Paths.get("foo", Array(): _*), this expression is not correctly typed but we have a deprecated implicit conversion from Array to immutable.Seq (that performs a copy of the array), for backward compatibility only, so that the expression becomes correctly typed.

@allanrenucci
Copy link
Contributor Author

allanrenucci commented Jul 12, 2018

class Test {
  def bar(xs: String*) = 1
  def foo(xs: Array[String]) = {
    bar(xs: _*)
    java.nio.file.Paths.get("", xs: _*)
  }
}

Simplified output from scalac Test.scala -Xprint:all (2.13.x branch):

[[syntax trees at end of                     typer]] // Test.scala
class Test extends scala.AnyRef {
  def foo(xs: Array[String]): java.nio.file.Path = {
    Test.this.bar((xs: _*));
    java.nio.file.Paths.get("", (xs: _*))
  }
}

[[syntax trees at end of                   uncurry]] // Test.scala
class Test extends Object {
  def foo(xs: Array[String]): java.nio.file.Path = {
    Test.this.bar(scala.runtime.ScalaRunTime.wrapRefArray[String](xs));
    java.nio.file.Paths.get("", xs)
  }
}

@julienrf As you can see typer does not introduce any implicit conversion from Array to immutable.Seq. uncurry will introduces a conversion but only if the method is not java defined

@julienrf
Copy link
Contributor

You’re right. We have a deprecated implicit conversion but it’s used only when one wants to coerce an Array into a scala.Seq (which is now immutable). The “splice operator” does not go through this machinery and, in the case of a Scala varargs method, wraps the Array into an immutable.ArraySeq without using the deprecated implicit conversion.

allanrenucci added a commit to dotty-staging/dotty that referenced this issue Jul 12, 2018
In expression position, if a tree is of the form `expr: _*`, try to type
`expr` as an `Array` and as a `Seq` if there are errors
allanrenucci added a commit to dotty-staging/dotty that referenced this issue Jul 12, 2018
In expression position, if a tree is of the form `expr: _*`, try to type
`expr` as an `Array` and as a `Seq` if there are errors
allanrenucci added a commit to dotty-staging/dotty that referenced this issue Jul 13, 2018
In expression position, if a tree is of the form `expr: _*`, try to type
`expr` as an `Array` and as a `Seq` if there are errors
@allanrenucci
Copy link
Contributor Author

We concluded that if Seq is an alias for immutable.Seq, you should not be able to pass a mutable array where a repeated argument is expected

@allanrenucci
Copy link
Contributor Author

I have opened scala/bug#11024

@allanrenucci allanrenucci added this to the 0.10 Tech Preview milestone Jul 23, 2018
@allanrenucci allanrenucci self-assigned this Aug 6, 2018
allanrenucci added a commit to dotty-staging/dotty that referenced this issue Dec 7, 2018
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).
allanrenucci added a commit to dotty-staging/dotty that referenced this issue Dec 7, 2018
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).
allanrenucci added a commit to dotty-staging/dotty that referenced this issue Dec 7, 2018
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).
allanrenucci added a commit that referenced this issue Dec 9, 2018
Fix #4785: Change typing rule for wildcard star args
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants