Search for type tests when matching abstract applied type

[nicolasstucki]

• Search for type tests when matching abstract applied type
• Normalize types when synthesizing type to avoid match type in the TypeTest closure signature.

Fixes #13433

[smarter]

Normalize types when synthesizing type test tp avoid isInstanceOf of a match type

What's the issue with isInstanceOf on a match type? And if there is an issue, shouldn't normalized be called at the point where we generate the isInstanceOf instead?

[nicolasstucki]

What's the issue with isInstanceOf on a match type? And if there is an issue, shouldn't normalized be called at the point where we generate the isInstanceOf instead?

Investigating further it was not the isInstanceOf, it was the type of the TypeTest.
We generated

{
  def $anonfun(s: Any): Option[(s : Any) & Matcher[String]] =
    if s.isInstanceOf[Matcher[String]] then
      Some.apply[s.type & Matcher[String]](
        s.asInstanceOf[s.type & Matcher[String]]
      )
      else None
  closure($anonfun:scala.reflect.TypeTest[Any, Matcher[String]])
}

but if we write it explicitly we infer

{
  def $anonfun(s: Any): Option[(s : Any) & String] =
    {
      if s.isInstanceOf[Matcher[String]] then
        Some.apply[s.type & Matcher[String]](
          s.asInstanceOf[s.type & Matcher[String]]
        )
        else None
    }
  closure($anonfun)
}

I will investigate further

[nicolasstucki]

As far as I can see it is only the synthesized version of the type test that needs this explicit normalization of the type.

[smarter]

By the way, is there any reason to limit ourselves to abstract types? One could imagine wanting to write a TypeTest instance for a concrete class with abstract type parameters:

import scala.reflect.Typeable

class Box[A](val a: A)(using val ta: Typeable[A])
object Box:
  given [A]: Typeable[Box[A]] with
    def unapply(x: Any) = x match
      case box: Box[_] =>
        if box.ta.unapply(box.a).isDefined then
          Some(box.asInstanceOf[x.type & Box[A]])
        else
          None
      case _ =>
        None

def func[A](box: Box[A]) = box match {
  case _: Box[String] => println("String")
  case _: Box[Int] => println("Int")
}

(this came up on gitter today: https://gitter.im/scala/scala?at=613a4ccf4c7be06b79abec3e)

[smarter]

(probably not something we want to change in this PR but worth discussing later)

[nicolasstucki]

By the way, is there any reason to limit ourselves to abstract types?

That is a limitation that I would like to remove at some point. The only noticeable thing is that some pattern matches that had unchecked warnings might become checked, which will change the semantics of some existing programs. Unless we do not provide any interesting implementations of TypeTest out of the box.

[smarter]

I will investigate further

Are you still investigating or is this PR waiting on me?

[nicolasstucki]

I will investigate further

Are you still investigating or is this PR waiting on me?

Waiting for you.

The investigation concluded with #13453 (comment). Sorry to not have been clear.

[OlivierBlanvillain]

That looks a bit suspicious. By default I would say that we need a really good reason to manually trigger type normalized. In principle all newly created types should already go thought nurmalization just after creation, and if fullyDefinedType creates non normalized types it might mean that there is a better spot in the code to do the normalization (deeper in the stack trace).

Note that we another place in the code where we do a fullyDefinedType call followed by a .normalized (which is equality suspicious):

https://github.com/lampepfl/dotty/blob/fe24fd3af2d985262b67394ac8d606a9830e0291/compiler/src/dotty/tools/dotc/typer/Synthesizer.scala#L163

My intition tells me that there is a way to both fix this issue and remove that .normalized in Synthesizer, and that this way would a better solution that the change done in this PR...

[nicolasstucki]

Then it seems to be a general limitation. We should probably open a separate issue for this improvement.

[nicolasstucki]

Rebased