Remove seen from TypeSizeAccumulator #20459
Conversation
This fixes scala#15692 and does not seem to break an existing compilation tests. The problem with seen when it comes scala#15692 is that seen means that type that has repeated types will get a lower size and this incorrectly triggers the divergence check since some of the steps involved less repeated types. The seen logic was introduced in scala#6329 and the motivation was to deal with F-bounds. Since not tests fail it not clear if this logic is still needed to deal with F-bounds? If it is still needed we can add a test and instead of removing the seen logic we can make it track only types the appear as a bound and could cause infinite recursion instead of tracking all.
|
I agree it would be good to think of an F-bounded test where the change proposed here would fail. I believe such tests exist, but they might be a bit tricky to find. I also agree that the current state, i.e. simply not counting types twice, is wrong. One thing we could do is have a (*) I say in principle, since these protecting LazyRefs are introduced only after the type is first constructed, in checkNonCyclic. But hopefully the check is done before we do an implicit search on a type. F-bounds are a curse, it would be so nice if we could get rid of them! |
This fixes #15692 and does not seem to break any existing compilation tests. The problem with seen logic is that give types with repeated types will get a lower size and this incorrectly triggers the divergence check since #15692 some of the steps involved less repeated types.
The seen logic was introduced in #6329 and the motivation was to deal with F-bounds. Since no tests fail it not clear if this logic is still needed to deal with F-bounds? If it is still needed we can add a test that fails and instead of removing the seen logic we can make it track only types the appear as a bound and could cause infinite recursion instead of tracking all.