Fix #11078: avoid widening F-bounds in type avoidance #11192
Conversation
liufengyun
changed the title
| @@ -462,6 +462,34 @@ object TypeOps: | |||
| else | |||
| range(defn.NothingType, defn.AnyType) | |||
| } | |||
|
|
|||
| /** Deviation from standard tryWiden: | |||
| * - Don't widen F-bounds | |||
There was a problem hiding this comment.
Maybe the standard tryWiden shouldn't widen f-bounds either?
There was a problem hiding this comment.
Yes, this is a trial to see how it works. It may incur a performance penalty.
If it works, I think we need a more systematic approach here: possible a field in SymDenotation to cache whether a symbol is F-bounded. The field can be set in checkNonCyclic, so we can avoid the computation here (or in ApproximatingTypeMap).
There was a problem hiding this comment.
F-bounded situations are already captured by LazyRefs. Maybe just add a case for LazyRefs to TypeOps.avoid?
Oh, I see you tried that already.
| @@ -438,6 +438,8 @@ object TypeOps: | |||
| tp.origin, fromBelow = variance > 0 || variance == 0 && tp.hasLowerBound)(using mapCtx) | |||
| val lo1 = apply(lo) | |||
| if (lo1 ne lo) lo1 else tp | |||
| case tp: LazyRef => | |||
There was a problem hiding this comment.
I think this should be emptyRange instead of TypeBounds.empty.
There was a problem hiding this comment.
Thanks for the tip, @odersky. I tried emptyRange, the test tests/run/i2895a.scala still fails. The problem is that LazyRef may leak into non-bounds places after asSeenFrom. It seems we need to stop earlier in the handling of bounds.
|
test performance please |
|
performance test scheduled: 42 job(s) in queue, 1 running. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/11192/ to see the changes. Benchmarks is based on merging with master (35bb073) |
|
@odersky Could you have a look at the last trial? Does it make sense? |
Fix #11078: avoid widening F-bounds in type avoidance