Fix #3015: use type inference to type child classes #3054
Conversation
scala#3054 has a better approach.
scala#3054 has a better approach.
| if (ref1.underlying <:< ref2.underlying) tp2.derivedSelect(ref1) else tp2 | ||
| case _ => tp2 | ||
| def instantiate(tp1: Type, tp2: Type)(implicit ctx: Context): Type = { | ||
| val approx = new TypeMap { |
There was a problem hiding this comment.
What does approx do? A more telling name or a comment would help.
There was a problem hiding this comment.
Indeed, a better name is more readable.
| val approx = new TypeMap { | ||
| def apply(t: Type): Type = t match { | ||
| case t @ ThisType(tref) if !tref.symbol.isStaticOwner && !tref.symbol.is(Module) => | ||
| newTypeVar(TypeBounds.upper(mapOver(tref & tref.givenSelfType))) |
There was a problem hiding this comment.
I think you want
newTypeVar(TypeBounds.upper(mapOver(t.underlying)))
here. That achieves what was written in simpler form. But why upper? Without an explanation it's hard to know whether it is correct. But it feels dangerous because the ThisType might appear at negative variance positions as well. How do you argue that the bound should not be changed in this case?
There was a problem hiding this comment.
The argument here is that the child type created from childSym.typeRef can only be of the shape path.Child, thus the ThisType is always covariant.
| if (protoTp1 <:< tp2 && isFullyDefined(protoTp1, ForceDegree.all)) protoTp1 | ||
| else { | ||
| debug.println(s"$protoTp1 <:< $tp2 = false") | ||
| NoType |
There was a problem hiding this comment.
Neat that this works! We might want to move it to something more generally available since it looks like the Linker will need something like this was well /cc @nicolasstucki. @DarkDimius
|
Can you rebase this PR on top of #3033? That would be the real test that exhaustiuveness checking is no longer tied to the particularities of how we encode applied types. I tried but got failures. |
|
test performance please |
|
performance test scheduled: 1 jobs in total. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/3054 to see the changes. Benchmarks for this PR is based on the merge of this PR (5ab6c36) with master |
scala#3054 has a better approach.
scala#3054 has a better approach.
scala#3054 has a better approach.
scala#3054 has a better approach.
scala#3054 has a better approach.
scala#3054 has a better approach.
Fix #3015: use type inference to type child classes