Add support for java records in pattern matching

[Florian3k]

Fixes #20561
Supersedes #24140

Implements pattern matching support for Java records.

• Record's fields are saved in ctx.base.javaRecordsFields (and temporarily as Attachment in JavaParsers, since symbols do not exists yet)
• Record class Rec used as an extractor pattern generates anonymous class with identity unapply method, which is then used in pattern matching. Existing static unapply in class takes precedence.
• Record class returned from unapply is treated similar to products, with all record's components being used in matching.

Have you relied on LLM-based tools in this contribution?

Yes, for codebase exploration.

How was the solution tested?

New automated tests

[SethTisue]

On the previous PR, Martin wrote:

I think whatever we do, this needs a spec change.

We should avoid a situation where we add something to the implementation and leave the spec change for later. So,
ideally, this PR is accompanied by a PR against the Scala spec. And the changes are either summarized or linked to here.

copying it here so it doesn't get forgotten.

afaics, wouldn't even need to be a separate PR, could be in the same PR, since the spec is here in-repo

[He-Pin]

Seems we can't have this in 3.9.x?

[Gedochao]

Seems we can't have this in 3.9.x?

We indeed can't have this in 3.9.x

[tanishiking]

I have a question, is there any performance difference vs the Java native one ...

@He-Pin I took the liberty of deleting your comment, so that no one can see inside the OpenJDK source code. Please do not paste a link to the OpenJDK source code, see:

https://github.com/scala/scala3/blob/main/CONTRIBUTING.md#forbidden

[He-Pin]

Sorry, can we make use of https://docs.oracle.com/en/java/javase/24/docs/api/java.base/java/lang/runtime/SwitchBootstraps.html on higher version of Java , which can generate smaller bytecode.

[lrytz]

This looks very good, thank you!

I have one question / discussion point about java records with a varargs parameters (see separate comment).

Claude found one bug: in pipelined compilation, there is no way to recover the record field names. The javaRecordsFields map is populated for parsed classfiles and Java source files. But in pipelined compilation, the record declaration comes from the tasty unpickler (the corresponding tasty was produced from the Java record AST in the previous compilation).

When unpickling the record class tasty, it seems we don't have a way to identify the primary constructor (there can be multiple constructors, it's not necessarily the lexically first), so we don't know what the field names are, and we cannot populate the javaRecordsFields map.

One solution around this is to store the field names in an internal annotation attached to the Java record class. This annotation is pickled to tasty, so it's still there in pipelined compilation. As a result, the global javaRecordsFields map would no longer be needed, the names can be read from the annotation.

That proposal is implemented on my branch.

Another observation: the synthetic class with the unapply method was only elided in the monomorphic case. Fix also in my branch.

Finally, one case that this PR implements is missing from the spec, the

object Baz:
  def unapply(b: Baz): JavaRecord = ...

It's a bit awkward to add it to the spec because option-less pattern matching is also not fully in the spec... Proposal also in my branch.

Branch is here, let me know what you think: https://github.com/lrytz/scala3/commits/pr26497/

[Florian3k]

@lrytz Good catches. I've looked at your branch. I'd pull all of it, but I'm a little wary of JavaRecordFields annotation. This is something I've tried before in #24140 and there was some pushback against it. I think we need to revisit this, get some more opinions.

[lrytz]

Yeah, the internal annotation is a bit unfortunate.

If we could figure out in Applications.scala / def javaRecordFields which is the primary constructor of a record class, we would not need to store the names anywhere. We'd have to identify the primary constructor in the classfile parser and in the java source parser, and make sure the marker we attach to the primary constructor is something that is also visible after Tasty unpickling. That's probably possible, I think Claude initially did that using the Synthetic flag, but I found that a bit too arbitrary. It could be an internal annotation, but then we can just as well put the record names into an annotation 🤷‍♂️

The problem with the global Map in Context is that it doesn't survive across Tasty.

[Florian3k]

Yeah, I'd go with this annotation if there aren't strong objections from the compiler team.

[SethTisue]

which is the primary constructor of a record class

I asked at core in what sense a Java record can even be said to have a "primary" constructor; primary constructors are a Scala-specific concept. For "the record" (ha ha ha!), I see now at https://docs.oracle.com/en/java/javase/22/language/records.html that Java records do have what the Java people call a "canonical" constructor:

A record class declares the following members automatically:
...

• A canonical constructor whose signature is the same as the header. This constructor assigns each argument from the new expression that instantiates the record class to the corresponding component field.
  ...
  You can define alternative, noncanonical constructors whose argument list doesn't match the record's type parameters. However, these constructors must invoke the record's canonical constructor

There is also new syntax for a "compact constructor", which makes constructors easier to write.

[Florian3k]

cc @lrytz
Changes:

• Changed the unapply to return tuple of components, removed the support for returning a record from unapply. Simplifies the implementation a bit I think, and reduces changes to the spec.
• Switched to annotations (cherry-picked your commit)
• Generate unapplySeq for vararg records
• Beta-reduce polymorphic unapply (cherry-picked your commit)
• Added test for matching null ((cherry-picked your commit)

[lrytz]

Changed the unapply to return tuple of components, removed the support for returning a record from unapply. Simplifies the implementation a bit I think, and reduces changes to the spec.

I think that's the right call.

Now we can consider to also simplify the spec: instead of defining record patterns in the spec, explain the shape of the synthetic unapply / unapplySeq methods that a Scala compiler adds to Java record classes. @sjrd wdyt?

[lrytz]

This wraps the array in an immutable Seq, which can be observed.

scala> val a = A("x", "y", "z")
val a: A = A[x=x, xs=[Ljava.lang.String;@2ca50ae3]

scala> val xs = a.xs
val xs: Array[String] = Array("y", "z")

scala> val sq = a match { case A(_, sq*) => sq }
val sq: Seq[String] = ArraySeq("y", "z")

scala> xs(0) = "buh"

scala> sq
val res0: Seq[String] = ArraySeq("buh", "z")

I'm not sure if we should keep it this way, or always clone the array. Should I ask in core?

[Florian3k]

Well, it'd be consistent with pattern matching on arrays.

scala> val xs = Array("x", "y", "z")
val xs: Array[String] = Array(x, y, z)
                                                                                                                    
scala> val ys = xs match { case Array(r*) => r }
val ys: Seq[String] = ArraySeq(x, y, z)
                                                                                                                    
scala> val zs = xs match { case Array(_, r*) => r }
val zs: Seq[String] = ArraySeq(y, z)
                                                                                                                    
scala> xs(1) = "buh"
                                                                                                                    
scala> ys
val res0: Seq[String] = ArraySeq(x, y, z)
                                                                                                                    
scala> zs
val res1: Seq[String] = ArraySeq(y, z)

But I think you can ask

[lrytz]

Right, that's implemented in UnapplySeqWrapper. So let's clone as well.

Perhaps we can have a similar wrapper? It would have to accept the initial non-varargs params as well. Or we keep the tuple and use array.toSeq.

[Florian3k]

Now we can consider to also simplify the spec: instead of defining record patterns in the spec, explain the shape of the synthetic unapply / unapplySeq methods that a Scala compiler adds to Java record classes.

I'm not so sure about this. Those methods are not visible to the programmer, they are synthesized on the go if we pattern match on a record. The fact that some methods are generated is more of a implementation detail imo.

[lrytz]

We don't need to make unapply/unapplySeq visible to users, but I think we could simplify the spec. Even if we leave "record pattern" as a concept in the spec, it could say "a record pattern behaves the same as an extractor pattern, given a synthetic definition of unapply or unapplySeq in the record class", something like that.

[sjrd]

We don't need to make unapply/unapplySeq visible to users, but I think we could simplify the spec. Even if we leave "record pattern" as a concept in the spec, it could say "a record pattern behaves the same as an extractor pattern, given a synthetic definition of unapply or unapplySeq in the record class", something like that.

That seems plausible, yes.

[Florian3k]

I think we would need then to explain all the possible shapes of unapply/unapplySeq (Boolean for empty record, Tuple for regular, Tuple with Seq for vararg, only Seq for vararg only), which doesn't really feel simpler to me.

Maybe we could say instead, that record pattern for record R[...](T1, ...Tn) behaves the same as constructor pattern for case class R[...](T1, ... Tn), including vararg parameter?

Anyway, in the end I'm fine with either option.