Make aliases of MatchAliases normal TypeAliases

[EugeneFlesselle]

Proposes to make isMatch true only for MatchTypes and higher-kinded abstraction of them.

As a result, code using isMatch to choose between a TypeAlias and MatchAlias will now use a TypeAlias when aliasing a MatchAlias. Which in turn allows for better de-aliasing, since dealias only de-aliases standard type aliases.

tryNormalize on AppliedType should only attempt reduction if there is an underlying match type. This could previously be identified by a MatchAlias tycon. We now need a recursive check.

Fixes #19821

[dwijnand]

Alternative to #19854

[dwijnand]

Here's two more places where I wonder whether we need change too:

1. The MatchAlias type test in TypeOps.simplify
2. The MatchAlias case in MatchType.InDisguise

Should those be if tp.isMatchAlias and case AliasingBounds(alias) if tp.isMatchAlias too?

[EugeneFlesselle]

@dwijnand For MatchType.InDisguise:

object InDisguise:
  def unapply(tp: AppliedType)(using Context): Option[MatchType] = tp match
    case AppliedType(tycon: TypeRef, args) => tycon.info match
      case MatchAlias(alias) => alias.applyIfParameterized(args) match
        case mt: MatchType => Some(mt)
        case _ => None
      case _ => None
    case _ => None

If I am not mistaken, the applyIfParameterized will only reduce a single application of a TypeRef tycon. So any cases that would match case AliasingBounds(alias) if tp.isMatchAlias which did not match case MatchAlias(alias) would then fail the second match in any case. So I think the cases matching MatchType.InDisguise remain the same with the changes.

I'm now wondering if you meant we might want to also accept these cases, independently of how they are affected by the aliasing changes ?

An equivalent change on main would then be

object InDisguise:
  def unapply(tp: AppliedType)(using Context): Option[MatchType] = tp match
    case AppliedType(tycon: TypeRef, args) => tycon.info match
      case MatchAlias(alias) => alias.applyIfParameterized(args) match
        case mt: MatchType => Some(mt)
         case MatchType.InDisguise(mt) => Some(mt)
        case _ => None
      case _ => None
    case _ => None

which doesn't seem to break anything.

Also, it looks like MatchType.InDisguise and underlyingMatchType would then be doing nearly the same thing, note that the latter uses superType which also does an applyIfParameterized. Maybe we only need one ?

[EugeneFlesselle]

For TypeOps.simplify, I agree it should be if tp.isMatchAlias instead.

[dwijnand]

Next is double-checking that we'll still function appropriately when we unpickle pre-3.4 tasty. I think it's dealt with the Unpickler change, right?

[EugeneFlesselle]

Next is double-checking that we'll still function appropriately when we unpickle pre-3.4 tasty. I think it's dealt with the Unpickler change, right?

The changes in Unpickler don't do anything more than factoring out the if alias.isMatch then MatchAlias(alias) else TypeAlias(alias) logic. Independently of the changes made in the Unpickler, it uses the updated isMatch method. So it'll unpickle aliases in a way that is consistent with what the rest of code base expects. But differently from what they were before being pickled prior to the changes, which I assumed (but am not sure of) was okay.

[EugeneFlesselle]

test performance please

[mbovel]

test performance please

[dottybot]

performance test scheduled: 148 job(s) in queue, 1 running.

[dottybot]

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/19871/ to see the changes.

Benchmarks is based on merging with main (a36849f)

[EugeneFlesselle]

Note that this PR is now based on #20077,
because we get the same issue with neg/i15183 when updating def isAnchor.