fix java array argument variance #1458
Conversation
|
May I know how this fixes #1456, where no array is involved in the example? |
| return if (type.psi is PsiArrayType) { | ||
| resolved | ||
| } else { | ||
| resolved.replace(type.element.typeArguments) |
There was a problem hiding this comment.
fix for #1456 is at this line. However introducing this line causes issues elsewhere due to array issue mentioned in this PR. As the issue on array turned out to be a bigger issue the PR is named after that.
| resolved | ||
| } else { | ||
| resolved.replace(type.element.typeArguments) | ||
| } |
There was a problem hiding this comment.
I'm a little bit confused. Why do you special-case Array here?
Also, I'm worried that replacing the type arguments in this way would effectively change the semantics of platform types.
There was a problem hiding this comment.
Replacing argument is because for java types, resolveJavaType can't return the correct argument variance as per illustrated in #1456 , special handling for array is caused from the main issue, where the flexible type returned from compiler is of invariant lowerbound, covariant upperbound, and trying to simply follow practice of replacing argument from reference element will cause issues in other places which is stated in the original description, therefore we keep it as is.
fixes #1456
It is a little bit messed up regarding flexible type.
By Java specs, array should have a single variance of covariance for its type arguments. However, from kotlin compiler, the said type is resolved to a flexible type with a lower bound of invariant type argument and an upper bound of covariant type argument. It might make sense from kotlinc since invariant is more restrictive hence works for a lower bound. It is just that when compiler is dealing with
KoltinType.arguments, it only looks at lower bound which causes trouble because the lowerbound argument of an array actually discarded the covariant part.It might look to be better respect Java spec by making it lower bound covariant as well, but attempting to do that caused a lot of failures elsewhere and trying to work against compiler seems a bad idea, following kotlinc's result is easier to implement here.
However it is still reasonable to add a special handling in reference element to reflect the covariant nature of array.