Spec for name based and irrefutable patterns

[martijnhoekstra]

Name based patterns were unspecced, and irrefutable patterns were underspecced.

This brings the spec beyond the current implementation, to where, I think, it should be.

Some-based irrefutable extractors and name-based extractors were part of scalac but unspecced, and are now part of the spec.

Name-based irrefutable extractors are pending per #9343

true-based irrefutable extractors may be broken/may have never worked per this comment: scala/bug#12232 (comment) and should probably be looked in to further.

[martijnhoekstra]

Name-based extractors for multiple parameters is not yet right.

[liufengyun]

FYI, Dotty also has some documentation about this: http://dotty.epfl.ch/docs/reference/changed-features/pattern-matching.html

[martijnhoekstra]

Thanks, I'll take a look @liufengyun

What I did wrong here is the n > 1 case for unapply with _1 to _n rather than a tuple.

[liufengyun]

Thanks, I'll take a look @liufengyun

What I did wrong here is the n > 1 case for unapply with _1 to _n rather than a tuple.

In Scala 3, it currently only checks ._1 and ._n for name-based matches. However, it checks T <: Product for product-based matches. If necessary, I guess Scala 3 can also adapt so that Scala 2 and 3 agree on the same protocol.

[martijnhoekstra]

@liufengyun Requiring T <: Product for extractors in scala 2 will likely break code in the wild in scala 2 and sounds to me like a non-starter as it will probably break code in the wild. As for what should happen in scala 3, dropping the product requirement would probably lessen the amount of migration friction.

The scala 2 compiler team at Lightbend and the dotty teams may want to discuss whether and how that should be aligned -- I'm just a random community passer-by trying to align the spec with the implementation and scala 3. When it's decision making time, it's up to the Powers that Be.

[liufengyun]

@liufengyun Requiring T <: Product for extractors in scala 2 will likely break code in the wild in scala 2 and sounds to me like a non-starter as it will probably break code in the wild. As for what should happen in scala 3, dropping the product requirement would probably lessen the amount of migration friction.

I'm not sure if we are talking about the same thing. By product-based matches, I mean this feature, which seems not supported by Scala 2. This might be a relief for the concern about migration.

[dwijnand]

Doesn't Name-based Match (which is supported already) subsume Product Match?

[liufengyun]

Doesn't Name-based Match (which is supported already) subsume Product Match?

They are different, Product Match is always irrefutable and does not go through the following interface used in name-based match:

{
  def isEmpty: Boolean
  def get: S
}

In product match, the return type of unapply is a product.

[dwijnand]

Ah, I see - thank you. Seems a little redundant, language-wise, but perhaps it's very practical and convenient. Do you know what the motivation for it was? (Sorry for the tangential discussion but I've been wanting to know what Scala 3 changed/added here.)

[liufengyun]

Ah, I see - thank you. Seems a little redundant, language-wise, but perhaps it's very practical and convenient. Do you know what the motivation for it was? (Sorry for the tangential discussion but I've been wanting to know what Scala 3 changed/added here.)

It's used in case classes, the unapply now can simply return the object, without boxing it in an Option and unboxing.

[dwijnand]

Ah right, yeah. It's good that the pattern matcher would, therefore, use the real, generated unapply, instead of how it accesses things directly on it in Scalac 2. 👍 thank you

[SethTisue]

should this still be marked as "draft"?

[martijnhoekstra]

@SethTisue yes, I still want to double-check some of it vs the implementation and scala3's implementation

[SethTisue]

remaining steps here, afaict:

• wait for Allow name-based extractors to be irrefutable #9343 to be merged
• final signoff from @liufengyun

[dwijnand]

Draft because of the PR dependency. Also needs a rebase it seems.

[dwijnand]

A fairly painful rebase... Only partially reviewed it, focussed more on just completing the rebase so we can review it together.

[dwijnand]

... so I might've made some mistakes rebasing, feel free to amend and push.

[martijnhoekstra]

Source wise nothing stands out as mis-rebased, but I haven't looked at the generated pdf and html yet.

[dwijnand]

OK, I plan on reviewing the change overall and getting it merged by Monday, Seth-time.

[martijnhoekstra]

I know Seth travels a lot, but I didn't realize they'd given him his own timezone. @liufengyun might still have something to comment about whether this is right from the dotty side of things too.

[liufengyun]

LGTM 👍

[dwijnand]

Fixed a rebase error in duplicating the n>1 section. LGTM, asking Seth if there are any spec-ese mistakes or any other improvements.