Skip to content

Conversation

@BobTheBuidler
Copy link
Contributor

@BobTheBuidler BobTheBuidler commented Sep 26, 2025

This PR is pretty simple, I just extended get_expr_length to work for a few more obvious cases:

  • builtins.enumerate
  • builtins.map
  • builtins.zip
  • builtins.range
  • builtins.list
  • builtins.tuple
  • builtins.sorted
  • builtins.reversed

This PR is ready for review. Are you going to want tests for all of these? I didn't want to spend time now until I know for sure.

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

BobTheBuidler and others added 5 commits September 25, 2025 20:54
This PR is pretty simple, I just extended get_expr_length to work for a few more obvious cases:

- `builtins.enumerate`
- `builtins.map`
- `builtins.zip`
- `builtins.range`

This PR is ready for review. Are you going to want tests for all of these? I don't want to spend time now until I know for sure.
@BobTheBuidler BobTheBuidler changed the title [mypyc] feat: extend get_expr_length for enumerate, map, zip, and range [mypyc] feat: extend get_expr_length for enumerate, map, zip, range, list, and tuple CallExpr Sep 26, 2025
@BobTheBuidler BobTheBuidler changed the title [mypyc] feat: extend get_expr_length for enumerate, map, zip, range, list, and tuple CallExpr [mypyc] feat: extend get_expr_length for enumerate, map, zip, range, list, tuple, sorted, and reversed CallExpr Sep 26, 2025
@BobTheBuidler
Copy link
Contributor Author

I added an IR test that covers a few of these, and I learned we need to merge #19935 before we can use it how I intended for the test. For now I'm leaving the test in just so it can be compared to #19935

The IR in #19935 is much cleaner, you'll see a r1 = PyTuple_New(5) which shows that the code from the PR did its job.

@BobTheBuidler BobTheBuidler changed the title [mypyc] feat: extend get_expr_length for enumerate, map, zip, range, list, tuple, sorted, and reversed CallExpr [mypyc] feat: extend get_expr_length for enumerate, map, zip, range, list, tuple, sorted, and reversed CallExpr [3/4] Oct 1, 2025
return not val

def test() -> None:
# this tuple is created from a very complex genexp but we can still compute the length and preallocate the tuple
Copy link
Collaborator

Choose a reason for hiding this comment

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

I can't see where the preallocation happens in the test case output. It seems to create a list and then convert it to a list?

@BobTheBuidler
Copy link
Contributor Author

BobTheBuidler commented Oct 14, 2025

I added an IR test that covers a few of these, and I learned we need to merge #19935 before we can use it how I intended for the test. For now I'm leaving the test in just so it can be compared to #19935

The IR in #19935 is much cleaner, you'll see a r1 = PyTuple_New(5) which shows that the code from the PR did its job.

@JukkaL FYI wrt the IR

Easiest path forward is probably merging this one first, given that #19935 proves that the specific change to our length logic IS successful, even though it doesn't show on the current IR test, and then letting me rebase #19935 on top of master which will correct the IR test from this PR when we merge it.

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.

3 participants