Skip to content

Change the signature of Array.unapplySeq for the benefits of Scala 3 #9977

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
wants to merge 1 commit into from

Conversation

smarter
Copy link
Member

@smarter smarter commented Mar 18, 2022

Fixes scala/scala3#14693, see discussion in the issue.

@scala-jenkins scala-jenkins added this to the 2.13.9 milestone Mar 18, 2022
@smarter smarter force-pushed the fix-Array-unapplySeq branch from e636e8e to 38f5cb7 Compare March 18, 2022 14:31
@smarter smarter changed the title Change the signature of Array#unapplySeq for the benefits of Scala 3 Change the signature of Array.unapplySeq for the benefits of Scala 3 Mar 18, 2022
@SethTisue SethTisue added the library:collections PRs involving changes to the standard collection library label Mar 18, 2022
@SethTisue SethTisue requested a review from a team March 18, 2022 14:39
@julienrf
Copy link
Contributor

julienrf commented Mar 21, 2022

IIUC, we change the signature of unapplySeq so that a typed pattern within a pattern sequence works. It seems to me that we are changing the type signature of unapplySeq to accommodate the internal way the Scala compiler handles such a combination of patterns.

What is the behavior of the following?

val xs = Array(1L, true)
val ys = Array(1L, 2L)

def matched(any: Any): Unit =
  any match
    case Array(x: Long, _*) => println(x)

matched(xs)
matched(ys)

I would expect both to match.

@smarter
Copy link
Member Author

smarter commented Mar 21, 2022

case Array(x: Long, *) => println(x)

This isn't valid syntax, I assume you mean:

case Array(x: Long, _*) => println(x)

What is the behavior of the following?

In Scala 3 before this PR, only the first matches. In Scala 3 after this PR, both match.

@smarter
Copy link
Member Author

smarter commented Mar 21, 2022

(and both still match in Scala 2 after this PR too)

Copy link
Contributor

@NthPortal NthPortal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any objection to this; at the same time, I'm not entirely convinced that the problem described by Julien lies here and not in the compiler

@smarter
Copy link
Member Author

smarter commented Mar 22, 2022

I'm not entirely convinced that the problem described by Julien lies here and not in the compiler

To restate what's going on: in general when typing the unapplySeq call we need to instantiate its type parameter as Any (especially in Julien's example, nothing else would fit). According to the specification, this means the pattern matching phase will generate a call to isInstanceOf[Array[Any]] which erases to instanceof Object[].
This PR changes the unapplySeq argument type so that we instead generate a call to isInstanceOf[Array[_ <: Any]] which erases to instanceof Object (because primitive arrays are subtypes of Array[_ <: Any] but not of Array[Any]).
I don't see what the compiler could be doing differently here, and I can't reproduce the Scala 2 compiler behavior because it relies on a compiler bug as mentioned in scala/scala3#14693 (comment).

@smarter
Copy link
Member Author

smarter commented Mar 23, 2022

in general when typing the unapplySeq call we need to instantiate its type parameter as Any

... is what I thought until now, but it turns out that in Scala 3 this behavior is specific to unapplies defined in Scala 2 classes, so this might in fact be a compiler bug, mea culpa :). Investigation in scala/scala3#14693 (comment)

@SethTisue SethTisue marked this pull request as draft March 24, 2022 01:01
@smarter
Copy link
Member Author

smarter commented Mar 24, 2022

Closing, superceded by scala/scala3#14766

@smarter smarter closed this Mar 24, 2022
@SethTisue SethTisue removed this from the 2.13.9 milestone Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
library:collections PRs involving changes to the standard collection library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missed match on Array(1L) in Scala 3.1.1
5 participants