Align with changes to name-based pattern matching in Scala 2.13.0-M5

[smarter]

See scala/scala#7068. Tentatively assigning to @OlivierBlanvillain since he wrote the current spec for name-based pattern matching in Dotty.

[allanrenucci]

Setting priority to blocker since this change is needed to compile and use 2.13 standard library. See the definition of scala.Array.unapplySeq https://github.com/scala/scala/blob/046937d99bfff4bcf261dcba836299578dae7478/src/library/scala/Array.scala#L523-L532

[Blaisorblade]

@nicolasstucki was also looking in that area for other reasons?

[allanrenucci]

In particular, this currently does not compile with Dotty:

object Array2 {
  // def unapplySeq[T](x: Array[T]): UnapplySeqWrapper[T] = new UnapplySeqWrapper(x)
  def unapply[T](x: Array[T]): UnapplySeqWrapper[T] = new UnapplySeqWrapper(x)

  final class UnapplySeqWrapper[T](private val a: Array[T]) extends AnyVal {
    def isEmpty: Boolean = false
    def get: UnapplySeqWrapper[T] = this
    def lengthCompare(len: Int): Int = a.lengthCompare(len)
    def apply(i: Int): T = a(i)
    def drop(n: Int): scala.Seq[T] = ???
      // ArraySeq.unsafeWrapArray(a.drop(n)) // clones the array, also if n == 0
    def toSeq: scala.Seq[T] = a.toSeq // clones the array
  }
}

class Test {
  def test(xs: Array[Int]): Int = xs match {
    case Array2(x, y) => x + y
  }
}

-- [E007] Type Mismatch Error: Test.scala:71:29 --------------------
71 |    case Array2(x, y) => x + y
   |                             ^
   |                             found:    Any(y)
   |                             required: String

[allanrenucci]

Can we really get rid of unapplySeq as suggested by @odersky in #4024?

Previously, such extractors used an unapplySeq whereas now they use an unapply (in the long term, we plan to get rid of unapplySeq altogether, replacing all its usages with unapply).

Consider this example:

class Person(val name: String, val children: Person*)
object Person {
  def unapply(p: Person) = Some((p.name, p.children))
}

class Test {
  def test(p: Person): Person = p match {
    case Person(_, c1) => c1
  }
}

Here c1 is of type Person. But now, if I write down the result type of the unapply:

class Person(val name: String, val children: Person*)
object Person {
  def unapply(p: Person): Some[(String, Seq[Person])] = Some((p.name, p.children))
}

class Test {
  def test(p: Person): Person = p match {
    case Person(_, c1) => c1
  }
}

-- [E007] Type Mismatch Error: Test.scala:84:26 --------------------
84 |    case Person(_, c1) => c1
   |                          ^^
   |                          found:    Seq[Person](c1)
   |                          required: Person
   |

c1 is of type Seq[Person]. The inferred type (Some[(String, Seq[Person] @Repeated)]) in the first example is not something that can be expressed in source code

[Blaisorblade]

1. FWIW: import scala.annotation.internal.Repeated (the type referenced in 4025987#diff-9d6330a6809ef256c9b6536358305ab3R1367) works and makes @Repeated work and the example compile. Technically, we'd have to make that annotation part of the public API (that is, moving it to scala.annotation.repeated). But all that seems a solvable design issue with a different form of extractors, so unrelated to this issue.

2. This issue is about implementing the 2.13 API with unapplySeq. It seems you're asking whether to implement it with unapply instead, but doesn't that have bigger compatibility problems?

[allanrenucci]

FWIW: import scala.annotation.internal.Repeated (the type referenced in 4025987#diff-9d6330a6809ef256c9b6536358305ab3R1367) works and makes @Repeated work and the example compile. Technically, we'd have to make that annotation part of the public API (that is, moving it to scala.annotation.repeated). But all that seems a solvable design issue with a different form of extractors, so unrelated to this issue.

I don't think annotations should affect the semantic of a program. We are also deprecating this different form of extractor, so no it's not unrelated.

This issue is about implementing the 2.13 API with unapplySeq. It seems you're asking whether to implement it with unapply instead, but doesn't that have bigger compatibility problems?

This issue is about supporting the same forms of name based pattern matching as scalac whether it is via unapply or unapplySeq. I am asking if this is feasible via unapply or if instead we decide not to get rid of unapplySeq.

I currently don't see how we can express repeated argument for name based pattern matching without unapplySeq. Let's consider the example above with Array2:

object Array2 {
  def unapply[T](x: Array[T]): UnapplySeqWrapper[T] = new UnapplySeqWrapper(x)

  class UnapplySeqWrapper[T](a: Array[T]) {
    def isEmpty: Boolean = false
    def get: UnapplySeqWrapper[T] = this
    def lengthCompare(len: Int): Int = ...
    def apply(i: Int): T = ...
    def drop(n: Int): scala.Seq[T] = ...
  }
}

def test(xs: Array[Int]): Int = xs match {
  case Array2(x) => x
}

Here the extractor is ambiguous. What's the extracted value x? Is it an UnapplySeqWrapper or is it the first element of the the Seq returned by UnapplySeqWrapper::toSeq? scalac makes this distinction with unapplySeq vs unapply

[Blaisorblade]

@allanrenucci I tend to agree: Those seem good reasons against both #4024 and dropping unapplySeq. Tho if we dropped unapplySeq we could as well use another name instead of get here.

But I see a bigger problem with renaming unapplySeq — which is actually not restricted to the standard library. We want to support a common Tasty output for Scala 2 and 3, but in such a Tasty what's the name of an extractor? We only use special names for compiler-generated ones, not for user-written names.

For both reasons, I'd implement unapplySeq here.

[Blaisorblade]

(EDIT: This is an attempted meeting summary): We'd absolutely have to keep unapplySeq for 3.0 (for migration), so we can support the Scala 2.12 API. unapplySeq is annoying in typer (because of the multiple attempts to typecheck) but removing it is not a priority.

We agree that @Repeated is problematic: we could hide it via type Repeated[T] = Seq[T] @Repeated but that'd still be a problematic first-class type that we don't want to support and spec.

[smarter]

Note also that there's still time to change how 2.13 behaves with respect to unapply/unapplySeq before release.