Partially fixes issue 8625 #8876
Conversation
|
Is there anything I can do to get this PR reviewed? |
|
|
||
| # Find the constraints on the type vars given that the result must be a | ||
| # subtype of the target type. | ||
| constraints = infer_constraints(ret_type, target_type, SUBTYPE_OF) |
There was a problem hiding this comment.
The relevant plugin hook gets called during semantic analysis, and solving constraints is unfortunately not supported during semantic analysis, since some type definitions may be incomplete. (This is pretty confusing since most simple cases work just fine, but this will fail in some edge cases.)
Basically this would need to be implemented without using any non-trivial type operations, perhaps by hard coding the behavior for a set of builtin collections.
There was a problem hiding this comment.
I am aware that when this gets called with a user defined converter with type variables the type definition is incomplete and doesn't contain enough information to actually solve the issue. However for builtin collections there is enough information available.
I started out by hard coding the behavior for a set of common builtin collections as you are suggesting, but soon realized that the code I was writing was identical to infer_constraints and that for builtin collections it works as long as I manually construct the list of type variables (type_var_ids = list({const.type_var for const in constraints})).
That's why this is a partial fix. But it since this works for builtin collections, it does address the most important use case. And for all other instances it fails no worse than before.
There was a problem hiding this comment.
@JukkaL Would this PR be acceptable for you if I duplicated the code of infer_constraints in a separate function to avoid any code coupling? As you say, we cannot solve this issue in general because when the plugin hook gets called type definitions may be incomplete, but we can solve this for this for builtin collections, and this PR does exactly that.
This fixes #8625 for instances when the converter is a builtin type like
typle,list, ordict.This is only a partial fix, since for user defined generic converters a similar issue still persists.
This partial fix however covers a very common and important use case.