Pattern matching on `Seq` in 2.13.0 unnecessarily unsafe?

#1
@ val Seq(a, b, c) = collection.mutable.Buffer(1, 2, 3)
scala.MatchError: ArrayBuffer(1, 2, 3) (of class scala.collection.mutable.ArrayBuffer)
  ammonite.$sess.cmd0$.<clinit>(cmd0.sc:1)

Ideally I’d want the Seq matcher to be able to handle mutable collections, but given that it can’t, does anyone know why it’s a runtime error and not a compile time error?

For example, the following fails at compile time:

@ val Seq(a, b, c) = 123
cmd1.sc:1: scrutinee is incompatible with pattern type;
 found   : Seq[A]
 required: Int
val Seq(a, b, c) = 123
       ^
Compilation Failed
#2

I think your code compiles because the compiler doesn’t know that collection.mutable.Buffer should never be subclassed to also extend immutable.Seq.

It’s similar to how the compiler accepts silly types like String with Int. This compiles fine in both Scala 2 and Dotty:

scala 2.13.0> def foo(x: String with Int) = { val s: String = x; val i: Int = x }
foo: (x: String with Int)Unit

Returning to your original code, I suggest you do your matching using collection.Seq:

scala 2.13.0> val collection.Seq(a, b, c) = collection.mutable.Buffer(1, 2, 3)
a: Int = 1
b: Int = 2
c: Int = 3

Note that for some reason, you can’t go up to Iterable:

scala 2.13.0> val xs = Iterable(1, 2, 3)
xs: Iterable[Int] = List(1, 2, 3)

scala 2.13.0> val Iterable(a, b, c) = xs
                  ^
              error: value Iterable is not a case class, nor does it have a valid unapply/unapplySeq member

I’m not sure if this omission is intentional or accidental. It seems accidental to me.

1 Like
#3

If you could patten match on Iterable, what order would it be in? I thought the difference between Seq and Iterable is exactly that Seq has a well defined order. To me, Iterable is useful when you are consuming or producing an unordered multi-set.

2 Likes
#4

Maybe the compiler should be more strict about destructuring than about pattern matching. If you want to destructure unsafely you can still cast explicitly which makes it clear that you’re doing something unsafe.

#5

Note that this is already done in Dotty, currently only under -strict mode, since https://github.com/lampepfl/dotty/pull/6389

[error] 13 |  val collection.immutable.Seq(a, b, c) = collection.mutable.Buffer(1, 2, 3)
[error]    |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]    |pattern's type scala.collection.immutable.Seq[Any] does not match the right hand side expression's type scala.collection.mutable.Buffer[Int]
[error]    |
[error]    |If the narrowing is intentional, this can be communicated by writing `: @unchecked` after the full pattern.
6 Likes
#6

That was my initial reaction as well. Iterable doesn’t guarantee stable iteration order.

But then I thought: regardless, you can iterate over an Iterable and it comes out in some order or another. So, why shouldn’t pattern matching be the same?

#7

Haha yeah it would be nice to get Dotty’s irrefutable patterns!

This was a bit of a pain for me because I hit it upgrading 2.12 -> 2.13, and suddenly had a bunch of runtime MatchErrors. Not sure how much other people pattern match on Seq, but perhaps it could be called out more loudly as a risk since unlike most of the other migration problems, these happen at runtime

7 Likes
#8

I agree, this is concerning.

Do the rewrite rules in the scala-collection-compat already do this rewrite? If they don’t, please open an issue or pull request in that repo.

Likewise, is this sufficiently covered (or covered at all) at https://docs.scala-lang.org/overviews/core/collections-migration-213.html ? If not, an issue or pull request would be welcome.

1 Like
#9

I think that’s because it would give false expectations to users. People would start relying on the order in which some iterables return their elements (as in case Iterable(0,x,y,z)), although in principle that order is totally unspecified, and could even change between two program runs or two versions of the JVM (HashSet is an Iterable and relies on hashCode).

3 Likes
#10

@LPTK Good point. Lord knows that happens often enough already.

Sorry Haoyi for sidetracking your thread a bit with the Iterable thing :slight_smile: