Don't lose info in bounds when pruning #8828
Conversation
New cyclic bounds can also appear as a consequence of unification. The latter goes through addOneBound, but not through addConstraint. So the prune logic that avoids cycles should go to addOneBound. ExprType checking could logically stay with addConstraint but it was simpler to move it as well.
Instead of checking that constraints are "separated" in addOneBound, we now ensure that they are.
The dropped lines were added as an intermediate stage before we settled on doing this in addOneBound.
Need to take into account type parameters that are not bound by constraint
Make sure cycles are detected and avoided in OrderingConstraint itself
Half of what it does is now handled in constraint itself
Without the additional case, scalatest fails.
This reverts commit f876537.
This reverts commit 16dec6b.
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/8828/ to see the changes. Benchmarks is based on merging with master (d7c0574) |
|
Exciting! I haven't had a deep look at the changes yet, but one thing I noticed last time I looked at the pruning logic is that it didn't handle cases like |
|
Hmm it's possibly the problematic unhandled form is actually |
Also detect cycles containing aliases and annotated types.
|
@AleksanderBG I assigned you as reviewer since knowing about the changes in ConstraintHandler should be useful for your planned GADT refactorings wrt AbstractContext. |
This reverts commit 16dec6b.
|
@AleksanderBG How about that review? I would like to merge this week. |
|
LGTM! Hearing your explanation on Monday about this change helped understand what's going on. Merging now. |
| .orElse(if (isUpper) defn.AnyType else defn.NothingType) | ||
| .orElse(if (isUpper) defn.AnyKindType else defn.NothingType) |
There was a problem hiding this comment.
@AleksanderBG Does this change remove the need for f94606a ?
There was a problem hiding this comment.
I was planning on testing that. I'm not completely sure how to make myself certain that now there's no need for f94606a - AFAIU, the above change will make the bounds of an unconstrained variable be >: Nothing <: AnyKind, but I'm not sure if it's not possible to still get a <: Any constraint, because other TypeBounds default to >: Nothing <: Any anyway.
There was a problem hiding this comment.
Ah right, those need to be fixed too, I have a wip branch wĥere I attempted that but I don't know when I'll be able to get back to it: https://github.com/dotty-staging/dotty/commits/wildcard-bounds
A refactoring of the code for constraints and constraint handling. No more solutions are lost when adding a bound to a constraint. This means that the only source of cutoff of the solution space is now
either, which is well understood.The improvements were made possible by systematically avoiding cyclic constraints in
Constraintitself."Cyclic" means that a type parameter
Aappears in its own bound at the toplevel (i.e. as a factor in some combination of&and|types. Any such occurrences can be replaced by the bottom or top types without loss of solutions. E.g.,