Imported From: https://issues.scala-lang.org/browse/SI-3968?orig=1
Reporter: @paulp

@paulp said:
Inspection reveals this is happening because java is burning us once again: scala is generating the instanceof operator, and according to the JVM an Array[String] is indeed an Array[Object].

I became uncertain and verified with the specification that this should not match. It seems pretty clear that it should not.

First, unlike arrays in Java or C#, arrays in Scala are not co-variant; That is, S <: T does not imply Array[S] <: Array[T ] in Scala. However, it is possible to cast an array of S to an array of T if such a cast is permitted in the host environment.
For instance Array[String] does not conform to Array[Object], even though String conforms to Object. However, it is possible to cast an expression of type Array[String] to Array[Object], and this cast will succeed without raising a ClassCastException. Example:

I believe I know how to fix it, and I will now investigate this belief.

@paulp said:
In case anyone's on the edge of their seat, I have this all fixed and there is good news and bad news. I lack the time for a full report right now so it'll have to be tomorrow. As a teaser, there are a number of places in the compiler expecting

  case x: Array[AnyRef] => 

to catch everything but the primitive arrays. So assuming that shouldn't match, what would we like to replace it with? The problem is that this:

  case x: Array[_] =>

Does not give you an Array[AnyRef] on the right hand side, so you end up with lots of currently unnecessary casts. It looks like a bug that:

  case x: Array[_ <: AnyRef] =>

doesn't work (it says "not found: type _$$1" or thereabouts.) However this seems to work, although I haven't properly tested. I can't get it to compile without defining an alias.

  type AnyAnyRef = T forSome { type T <: AnyRef }
  case x: Array[AnyAnyRef]   => ...

@paulp said:
Also, the patch at a point where everything works approximately as I expect is available here:

http://github.com/scala/scala/commit/927db662c607160bdc8ddb17dd96637516bcd821

The test case it passes:

http://github.com/scala/scala/blob/927db662c607160bdc8ddb17dd96637516bcd821/test/files/run/array-match-variance.scala

@adriaanm said:
(note: this is pending for discussion at a meeting where Martin is present, probably next week)

@paulp said:
Replying to [comment:6 moors]:

(note: this is pending for discussion at a meeting where Martin is present, probably next week)

OK, thanks for the info. As a point of interest this appears to have been the true cause of various confusions we experienced regarding the necessity of matchesPattern etc.

@adriaanm said:
the compiler should report an unreachable code error for the example above, but the matching behaviour has keep following the semantics of Java's instanceof, such is the oath we took when embarking on this journey

even if we did make a u-turn onto the high road, the worry is we'd be stuck in reflection traffic

@paulp said:
Replying to [comment:10 moors]:

the compiler should report an unreachable code error for the example above, but the matching behaviour has keep following the semantics of Java's instanceof, such is the oath we took when embarking on this journey

I think this needs further consideration. Can we use spec citations? I missed the oath apparently.

even if we did make a u-turn onto the high road, the worry is we'd be stuck in reflection traffic

I can't see how it's a u-turn. The example above doesn't now and hasn't ever said unreachable code. But if we intentionally match covariantly it remains trivial to create runtime failures. How is it any different in effect to have Array[String] match as Array[AnyRef] in a pattern match than it is to allow Array[String] as an argument to an Array[AnyRef] parameter type?

I can't see any point in making arrays behave invariantly everywhere else and then leaving open this huge hole for runtime failure.

(Array[String]("a"): Any) match { case x: Array[AnyRef] => x(0) = new AnyRef }
java.lang.ArrayStoreException: java.lang.Object

And if we're really going to make matching work right wrt type parameters when a manifest is available, we have this fairly ridiculous situation where the erased types match correctly and the type which actually reified the relevant type information has that information discarded and leads to a runtime failure.

@paulp said:
]And I realize we could take the position that case x: Array[String] is "unchecked" the way it is with say List. But it just seems terribly perverse to act like it's unchecked when it's only unchecked because we refuse to check, not because we can't. (Also, it doesn't now and I do not think ever has give an unchecked warning.)

@paulp said:
OK, I see it in the spec now, although it seems awfully quiet.

A parameterized type pattern scala.Array[T1], where T1 is a type pattern. This type pattern matches any non-null instance of type scala.Array[U1], where U1 is a type matched by T1.

@paulp said:
Noting for the record that martin has approved fixing this for pattern matching (with isInstanceOf doing the looser "java" match, which is consistent with the other places where the pattern matcher is tighter such as path dependent types) and I just need a little time to finish it. Hopefully whatever follows 2.9.

@soc said (edited on Dec 1, 2011 8:04:09 PM UTC):
Running this on

scala -Yvirtpatmat
Welcome to Scala version 2.10.0.r26093-b20111130020250 (OpenJDK 64-Bit Server VM, Java 1.7.0_147-icedtea).

Results in:

scala> Test.main(null)
3
3

This got worse, didn't it?

Update: Weird, same behavior also with the old patter matcher ... is this a regression?

@adriaanm said:
null is not an array

@adriaanm said:
so this is really a triple-whammy bug:

1. the spec should be updated, as currently I read it as specifying covariant matching for arrays:
   "A parameterized type pattern scala.Array[T1], where T1 is a type pattern.
   This type pattern matches any non-null instance of type scala.Array[U1], where U1 is a type matched by T1."

"where U1 is a type matched by T1" should really read "where a value of type U1 is matched by the type T1"
(this is consistent with the uses of "a value matched by a type" in the other bullets of 8.2)

2. implement invariant matching for arrays (including updating the analyses)

3. implement support for bounded existentials in type patterns

I agree all of these are worthwhile, but I don't think I can do this in time for 2.10.0.
Thus, demoting to major.

@gkossakowski said:
Unassigning and rescheduling to M7 as previous deadline was missed.