confusing doc page on option-less pattern matching - strange implicit in unapply example

[bjornregnell]

Problem

In this document page:
https://github.com/lampepfl/dotty/blob/main/docs/_docs/reference/changed-features/pattern-matching.md

The initial definition of what an extractor is says that it is a an object having a special unapply looking like this:

def unapply[A](x: T)(implicit x: B): U
def unapplySeq[A](x: T)(implicit x: B): U

Why is there an (implicit x: B) ? And what is U?

The doc page never explains it and I can't find any place in the examples further down where the mystic implicit of type B is used. And if it must be there, should this not be a using as this is Scala 3?

Proposal

If the implicit is there just to illustrate that there can be an implicit then I think it is better to remove it and just state in the text that a using-parameter is allowed.

[som-snytt]

I'd suggest "the most general form is", and note that the type param and the second parameter section are optional.

Also, "The different kinds of extractors are characterized by the result type U."

[bjornregnell]

Aha, so the implicit param is there just to show that it can be there - but what is it good for? And what is the type B? And where is the type T? (I guess it should be A?) And I still think it should be a given if it must be there? And why x two times - that seems like a compile error ("x is already defined as parameter x")

I think it would be better to start off with:

def unapply[A](x: A): U
def unapplySeq[A](x: A): U

As the above is the normal thing and later then say that a using-part is also possible if needed (but needed for what? And what is B? And we need another name than x for that second mystic using.

def unapply[A](x: A)(using y: B): U
def unapplySeq[A](x: A)(using y: B): U

"The different kinds of extractors are characterized by the result type U."

Thanks! This explanation helps a lot to grok why U is there and now the following text makes much more sense. But maybe C instead of U to follow the A B C pattern of type names.

[som-snytt]

Oh, good catch! https://discord.com/channels/632150470000902164/632628644035952680/1025685953966256148

on the param name!

They used T for the "type" of the scruTinee, so U for the oUtput. My objection would be that U as a result often (Akka Future style) means "this arbitrary type param is effectively Unit because we ignore the value".

Otherwise, I always prefer simple A, B, C. User examples with class A are like "nails on a chalkboard." I think R is used in the explanation, otherwise I'd suggest it. Oh, maybe type X for "EXtraction"!

The reason to show the general signature is that you wonder "will it detect my syntax"? For example, with case classes, it won't generate an apply in the companion if you defined one. Does that include my def apply[A]? And this feature is all about the signature.

Any implicit clause or type param would be for benefit of the implementation on the RHS, which the text could make plain.

[bjornregnell]

Thanks @som-snytt -- happy if you want to coach me here in this PR, to further improve my attempt to fix this: #16132

[Sporarum]

Hello,
I also noticed that when reading the whole Reference, and I included a solution with others:
#15993

As the above is the normal thing and later then say that a using-part is also possible if needed

I did pretty much exactly that:
https://github.com/lampepfl/dotty/pull/15993/files#diff-275787e7746511aaf8363ec8180f75e39bd2a1b3c695f5277c4e960260881217R13-R24

[bjornregnell]

Aha @Sporarum - sorry for not finding that one. Your correction kept the unkonwn T though (not easy to spot for a human who interprets everything as what it should be :) ). How do you want to proceed - there will be a merge conflict of both our PR:s are merged... Perhaps you could split your PR into several ones, one er file or something and then we could manually merge our contributions on patmat?

[bjornregnell]

Just saw your comment over at #16132
I just closed that. Feel free to be inspired :) @Sporarum

[Sporarum]

Thx, I'll be sure to fix the unknown T ^^

[Sporarum]

I had forgotten to close this issue when #15993 was merged, I will therefore do it now

Rest assured your feedback has been taken into account, if you think something is still confusing or incorrect, either reopen this issue or open a new one ^^