Preserve TypeLambdas in wildApprox #6271
Conversation
wildApprox should not replace TypeParamRefs which are bound by TypeLambdas which are contained in the type being approximated. Fixes #i6238.
|
test performance please |
|
This looks like its on the right track. Let's see what the performance tests say. |
|
performance test scheduled: 2 job(s) in queue, 1 running. |
| case tl: TypeLambda => | ||
| val internal1 = internal + tl | ||
| val paramInfos = tl.paramInfos | ||
| val paramInfos1 = tl.paramInfos.mapConserve { |
| case _: HKTypeLambda => HKTypeLambda(tl.paramNames)(tl1 => paramInfos.map(substBounds(tl1)), tl1 => res1.subst(tl, tl1)) | ||
| case _: PolyType => PolyType(tl.paramNames)(tl1 => paramInfos.map(substBounds(tl1)), tl1 => res1.subst(tl, tl1)) | ||
| } | ||
| } |
There was a problem hiding this comment.
I think all this machinery is already provided by derivedLambdaType. Can you try this instead as the case for TypeLambda?
val internal1 = internal + tl
val paramInfos1 = tl.paramInfos.mapConserve {
case tb @ TypeBounds(lo, hi) =>
tb.derivedTypeBounds(
wildApprox(lo, theMap, seen, internal1),
wildApprox(hi, theMap, seen, internal1)
)
}
val res1 = wildApprox(tl.resultType, theMap, seen, internal1)
tl.derivedLambdaType(paramInfos = paramInfos1, resType = res1)There was a problem hiding this comment.
Damn. That's what I expected, and I looked for it, but somehow managed to miss newLikeThis and then ended up rewriting it inline :-(
There was a problem hiding this comment.
It can even be simplified further, it seems. See 85c641f.
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/6271/ to see the changes. Benchmarks is based on merging with master (95fd03e) |
- Use derivedLambdaType - Drop parameters of PolyTypes altogether
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
@odersky I'm fine with your simplifications wrt |
We always reduce PolyTypes to their result type, inferring type parameters. Since the type parameter references of PolyTypes are eliminated anyway by wildApprox, there's no need to keep the parameters around. That's the thinking behind it, anyway. |
Right, but these type parameters are being bound internally to the type being approximated, so do they really need to be wildcarded out? My understand was that the point of the approximation was only to eliminate references to type parameters bound externally to the type being approximated. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/6271/ to see the changes. Benchmarks is based on merging with master (2f557b5) |
I think it could go either way, but I did not find an instance where we would actually call |
I think it could go either way, but I did not find an instance where we would actually call wildApprox on a PolyType. So it's better to simplify and treat PolyTypes just like other TypeLambdas.
|
This LGTM ... happy with this being merged if you are :-) |
wildApproxshould not replaceTypeParamRefswhich are bound byTypeLambdaswhich are contained in the type being approximated.Fixes #6238.