Fix assertion failure on nested tuple identity

[Praetonus]

This fixes a bug introduced in 9b9c084 caused by the __is method of a tuple nested in another tuple not being reached.

This should only get a changelog entry if we do a new release before merging this, since the bug exists only in the unreleased version.

[plietar]

I'm not sure I get the logic of determining reachability by considering the contents of both sides.

If we determine that the TK_IS needs the __is method, (ie subtype_kind_overlap contains SUBTYPE_KIND_TUPLE), then I think we should only mark the method as reachable on the left hand side type, and all types it contains recursively. We should ignore completely the right hand side type.

The reachability logic here should match the genident one, especially the box_is_box and gen_is_tuple_fun methods

[plietar]

Tuples could have different length, in which case this asserts (or segfaults)
#1946

[Praetonus]

I'll fix that.

[plietar]

Doesn't add_rmethod already take care of adding it to all subtypes ?

[Praetonus]

It does, but it doesn't handle the case of a tuple subtype that should add __is to its members.

[plietar]

Indeed, thanks

[Praetonus]

@plietar The __is method is only needed when both the left hand side type and the right hand side type of an is expression have the same tuple type as a subtype.

For example, consider this case:

a: ((U8, U16) | (U16, U16))
b: ((U8, U16) | None)

a is b

Here, both a and b can be an (U8, U16) tuple, but only b can be an (U16, U16) tuple. This means that (U8, U16) needs the __is function but not (U16, U16) since we statically know that if a has that type, there is no need to check the contents of the tuple because b will have a different type.

If we were to determine the reachability of __is based on the left hand side type, we'd always get all needed types (since they'd also be contained in the right hand side type) but also a few unneeded types, like (U16, U16) here.

[plietar]

Shouldn't this case call add_rmethod as well ?

[Praetonus]

No, we only need __digestof when using digestof on an abstract type (i.e. not a tuple) with boxed subtypes.

[plietar]

What I meant here is that when generating the __is method, we consider the two types to be identical (see #1950, this isn't ideal, but that's how it is right now).

Therefore we should only recurse considering the left hand side type, ignoring the right one.

Additionally right now we generate the __is on both sides, but we only need it for the left, since this is what we'll be calling during codegen.

[Praetonus]

I do think that recursing on every subtype of the left hand side type is unnecessary and marks too many types. Recursing on types which are subtypes of both the left hand side type and the right hand side type marks every needed type and no unneeded type. I expect that we'll have to change that logic in order to fix #1950 but for now I think it's fine.

I'll fix the __is generation, generating both sides is indeed unnecessary.

[Praetonus]

Conflicts fixed, merging.