Fix #3544: Better cache invalidation of uninstVars #3726
Conversation
Type variables can be instantiated without changing the underlying constraint. We need to check that this has not happened before returning a previously cached uninstVars.
| @@ -548,7 +548,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, | |||
|
|
|||
| /** The uninstantiated typevars of this constraint */ | |||
| def uninstVars: collection.Seq[TypeVar] = { | |||
| if (myUninstVars == null) { | |||
| if (myUninstVars == null || myUninstVars.exists(_.inst.exists)) { | |||
There was a problem hiding this comment.
Shouldn't this use instanceOpt instead of inst? Otherwise this will still return instantiated type variables whose inst field has not yet been set.
There was a problem hiding this comment.
I wonder if it wouldn't be simpler and more efficient to clear the cache everytime we instantiate a typevar instead.
There was a problem hiding this comment.
Anything in the constraint is already taken into account in uninstVars. After all, constraints are immutable. So we only need to make sure there are no instantiations not mirrored in the constraint. We can't clear the cache when we instantiate a typevar because there is no 1-1 mapping between constraints and typevars. So there could me multiple constraints that become invalid.
Type variables can be instantiated without changing the underlying constraint.
We need to check that this has not happened before returning a previously
cached uninstVars.
Also fixes #3470.