Fix #8647: Remove TypeParamRef from instantiated test #8704
Conversation
| // `arg1` and `arg2` are fully instantiated. | ||
| def fullyInstantiated(tp: Type): Boolean = new TypeAccumulator[Boolean] { | ||
| override def apply(x: Boolean, t: Type) = | ||
| x && { |
There was a problem hiding this comment.
I don't understand the comment above: "We can only trust a "no" from isSameType when both arg1 and arg2 are fully instantiated.". What does "trust" mean here? isSameType(tp1, tp2) returning false means "I wasn't able to prove that tp1 and tp2 are the same type", having or not having type variables in these types does not change that.
There was a problem hiding this comment.
It means we interpret !isSameType(A, B) as "A and B are definetly different type" when the conditions in && { ... } are met. The alternative would be to write another recursive method provablyNotSameType in the same style as provablyDisjoint that would be used here to draw disjointness conclusions from invarant type parameters.
There was a problem hiding this comment.
It means we interpret !isSameType(A, B) as "A and B are definetly different type"
Unfortunately that's not true, even if you exclude type variables, type parameters and abstract types. For example Nothing and Int & String are the same type, but the compiler doesn't know that and isSameType will return false.
The alternative would be to write another recursive method provablyNotSameType in the same style as provablyDisjoint that would be used here to draw disjointness conclusions from invarant type parameters.
That seems like a good way forward yeah.
There was a problem hiding this comment.
The alternative would be to write another recursive method provablyNotSameType
It looks like scalac has something like this for deciding if it needs to emit an unchecked type test warning, this could be a good inspiration: https://github.com/scala/scala/blob/dfb8f7364a56fc01ad1eaea57980fba586ef6914/src/compiler/scala/tools/nsc/typechecker/Checkable.scala#L275
(we may in fact want to use this in our own unchecked type tests which are currently pretty broken: #8808)
smarter
left a comment
smarter
left a comment
There was a problem hiding this comment.
I think this is worth getting in since it's a net improvement even if it needs more work.
2 participants
It appears that
TypeParamRef-s that appear in types insideprovablyDisjointalways come from local type binders. As a result, it's OK to consider types containingTypeParamRef-s to befullyInstantiatedand trust the result ofisSameTypeat that point.