Skip to content

Conversation

@BobTheBuidler
Copy link
Contributor

@BobTheBuidler BobTheBuidler commented Sep 26, 2025

This PR extends get_expr_length to work with type information from RTuple and TupleType classes

All of the get_expr_length PRs are entirely independent and can be reviewed/merged in any order.

@BobTheBuidler BobTheBuidler changed the title [mypyc] feat: extend get_expr_length to work with RTuple and TupleType [mypyc] feat: extend get_expr_length to work with RTuple and TupleType Sep 26, 2025
@BobTheBuidler
Copy link
Contributor Author

If we merge #19931 first, this PR will be reflected in a change to its IR test data

@BobTheBuidler BobTheBuidler changed the title [mypyc] feat: extend get_expr_length to work with RTuple and TupleType [mypyc] feat: extend get_expr_length to work with RTuple and TupleType [2/4] Oct 1, 2025
rtype = builder.node_type(expr)
if isinstance(rtype, RTuple):
return len(rtype.types)
proper_type = get_proper_type(builder.types[expr])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this or will the check above it catch every case?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is redundant. If this would be required for some unexpected reason, the issue should be fixed in node_type instead of here, since it would likely affect other things as well. Can you remove the second check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

r4 = box(int, r1)
r5 = box(int, r2)
r6 = box(int, r3)
r7 = PyTuple_Pack(3, r4, r5, r6)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, the new IR is much more efficient!

@JukkaL JukkaL merged commit d69419c into python:master Oct 14, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants