Emit deferred reachability warnings & fix boxing/unboxing adapting

[dwijnand]

Emit deferred reachability warnings
There are three kinds of unreachable cases:

1. the cases that are provably disjoint from the scrutinee type
2. the cases that are not subtypes of the scrutinee type
3. the cases that are covered by a previous cases, aka an overlap

The first one should be (I didn't review or stress test it) handled when
type-checking the pattern.

The third one is the safest, in terms of not emitting false positives to
the user, because it's within the conservative approximation of the
scrutinee space and the previous cases spaces.

The second one is where it gets tricky, because we don't know they're
part of the scrutinee type but we also don't know they're not part of
the scrutinee type. Erasure is the simplest example: if the scrutinee
type is type ThisTree <: Tree then if you write cases for subtypes of
Tree that aren't subtypes of ThisTree, then they're not subtypes nor are
they provably disjoint from the scrutinee type.

Martin mentioned that it's been tried a few times to restrict patterns
to subtypes, but that it's been found too restrictive and provably
disjoint is the right choice for type errors.

So the logic that I recently reimplemented was to only emit reachability
warnings after a reachable a case was found, therefore avoiding to emit
any warnings in the ThisTree case, emitting for all the overlap
unreachable cases.

The problem was that accidentally introduced a dependency on the
ordering of the cases, where unreachable initial cases aren't warned but
reordered they would. So I hold onto those references and unbuffer as
soon as we find a provably reachable case (covered != Empty) or a
provably unreachable case (prev != Empty).

Also, I've had separate conversations with Seth and Lukas about whether
match analysis should just widen those non-class types, but it's also
plausible that a user has created a whole type hierarchy with type
members and would prefer for it to not discard it all by widening to
class/object types, but instead reason within the bounds of the given
scrutinee type.

---

Emitting deferred reachability warnings (the previous commit) caused
tests/patmat/boxing.scala to start failing, because my logic on boxing
and unboxing wasn't actually right.

So this fixes how the case type (tp1) gets adapted for the scrutinee
type (tp2), with two examples in the comments from that boxing test
case. Also organise that constant types are converted if boxing isn't
involved.

[smarter]

The main commit in this PR just says "Emit deferred reachability warnings" but doesn't change any test so it's not clear to me what it does, it'd be good to have a more detailed commit message and tests in the same commit if possible.

[liufengyun]

LGTM 👍

[liufengyun]

Just curious: I'm wondering if it is possible to push the 2nd <:< tp2 into adaptType(tp1, tp2) to make the new "subtyping" rule more explicit? (I haven't thought through the details, please ignore if it does not make sense technically).

[dwijnand]

I can't tell if I'm being cautious or just short-sighted and bone-headed, but I kind of don't want to get into the business of trying to integrate the type-fudging we're doing here, tightly into the subtyping rules. Re-reading your earlier "augment the rules" comment, realising it's what you meant, rather than this. We tried an earlier suggestion from Guillaume of using "isPrimToBox" alone, but that only considers the classSymbol alone, incorrectly making Integer <:< (1: Int) true. So maybe the "augment" idea isn't a good idea because the subtyping rules are already so complex? Or, again, I'm being short-sighted.

Adapting types has a lot of precedent in the codebase, such as in typer's tree adapting (where I lifted the ConstantType converting code) but also in TypeOps where there's a lot of type mapping to approximate things and try subtyping multiple times.

In many ways the subtyping rules are fixed, but I get the sense that even though fundamental changes don't happen often, there are changes in encoding or implementation detail that put all these auxiliary type mapping and approximating code paths at risk of diverging from their original intent (when unguarded by tests).

WDYT? Am I being too paranoid? Do you think it's reasonable easy for me to just sit down for a second and figure out how to get these silly cases to give the right answer?

[liufengyun]

I kind of don't want to get into the business of trying to integrate the type-fudging we're doing here, tightly into the subtyping rules

Sorry I wasn't clear. I don't mean change <:<. What I was proposing is some refactoring. Instead of tp1 <:< tp2 || adapt(tp1, tp2) <:< tp2, what about tp1 <:< tp2 || primitiveSubtype(tp1, tp2)?

As I mentioned, this is a minor thing, more of a style --- the current PR is good as it is.

[dwijnand]

Oh I had understood that. By integrating I meant primitiveSubtype must be some subset of the subtyping rules, which at least handles that Integer <:< (1: Int) is false.

Request addressed