SI-6611 Tighten up an unsafe array optimization

[retronym]

The net was cast too wide and was unsafely optimizing away array
copies.

review by @paulp

I'd like to move the following to Definitions, but I'm not what I can use in there to pick a particular overload when arity isn't enough.

ArrayModule_overloadedApply.suchThat {
  _.tpe.resultType.dealias.typeSymbol == ObjectClass  // [T: ClassTag](xs: T*): Array[T] post erasure
           })

One idea I had was to annotate such methods:

object Array {
  @scala.annotation.internal.magic("Array_apply_ref")
  def apply[T: ClassTag](args: T*)

It would simplify the compiler logic and make it more self evident in the source that this method is treated specially by the compiler. But maybe they should just be macros, which would be even more revealing.

Relates to SI-6247, in that it shows where one overload of Array.apply is already optimized, and where the corresponding primitive optimizations could go (again, unless macros are preferred.)

[scala-jenkins]

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1544/

[scala-jenkins]

jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1544/

[scala-jenkins]

Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/836/

[scala-jenkins]

jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/836/

[paulp]

I don't like this style of extractor; I think it should return Some(tree) when it's a different tree, and None otherwise. Otherwise you learn nothing from the fact that it matched. I have a work in progress with a bunch of extractors along these lines: https://github.com/paulp/scala/blob/value-class-extractors/src/compiler/scala/tools/nsc/Global.scala#L1118

[paulp]

Here are some less value-class-specific ones: https://github.com/paulp/scala/blob/value-class-extractors/src/reflect/scala/reflect/internal/Definitions.scala#L1122

[retronym]

I find that Some returning extractors are useful additions to a pattern vocabulary. Particularly handy is: object && { def unapply[T](t) = Some((t, t)) }.

In this case I'm relying on it matching in both of the trees:

// Array(null: Object)
scala.Array.apply(scala.this.Predef.wrapRefArray(Array[java.lang.Object]{(null: java.lang.Object)}), reflect.this.Manifest.Object());

// Array(null: String)
scala.Array.apply(scala.this.Predef.wrapRefArray(Array[java.lang.String]{(null: java.lang.String)}.$asInstanceOf[Array[java.lang.Object]]()), reflect.this.ClassManifest.classType(classOf[java.lang.String]));

The different approaches would result in these patterns:

Apply(appMeth, List(Apply(wrapRefArrayMeth, List(arg @ StripCast(ArrayValue(_, _)))), _))
Apply(appMeth, List(Apply(wrapRefArrayMeth, List(arg @ (AsInstanceOf(ArrayValue(_, _), _) | ArrayValue(_, _))), _))

Both extractors are situationally useful; even though AsInstanceOf is more fundamental.

[paulp]

I see your point, though still unexcited about it. I think ideally the core logic of any extractor which involves a runtime check should have a "None" and "Some" components. Behavior like you want here I would wish to put in an "extractor combinator" (which I'm not at all sure is workable) which takes an unapply(x: T):Option[T] and transforms it into (x: T):Some[T] by tacking on "orElse Some(x)". Anyway, I was just throwing it out there, I have complete faith in the legitimacy of your reasons for doing things.

[paulp]

BTW, ".tpe.resultType.dealias.typeSymbol" is redundant, .typeSymbol dealiases.

[paulp]

So in summary, LGTM.

[scala-jenkins]

Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/836/

[scala-jenkins]

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1544/

[scala-jenkins]

jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/836/

[scala-jenkins]

jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1544/

[retronym]

I cleaned this up a little ways and extended this optimization for primitive arrays. Encore review, please, by @paulp.

[paulp]

I'm just a simple country hacker, but LGTM.