[mypyc] feat: ForFilter generator helper for `builtins.filter` #19643

BobTheBuidler · 2025-08-12T02:16:59Z

This PR adds a ForFilter helper class for for loops over builtins.filter which allows mypyc to maintain control of the function calling, so it can call native functions and use primitive ops, and to optimize filter's boolean check based on func's return value type

for more information, see https://pre-commit.ci

mypyc/irbuild/for_helpers.py

…to for-filter

for more information, see https://pre-commit.ci

BobTheBuidler · 2025-08-12T13:15:04Z

Just realized I can extend this class to make it work for both builtins.filter and itertools.filterfalse with only 4 more LOC

I wouldn't think to make a primitive for iteration solely over itertools.filterfalse, but considering it would share the same object and only require one more boolean attribute, I think it makes sense to support here. What do you guys think?

…to for-filter

for more information, see https://pre-commit.ci

…to for-filter

This reverts commit c9680dc.

for more information, see https://pre-commit.ci

…to for-filter

BobTheBuidler · 2025-08-13T06:50:34Z

I went ahead and implemented the above on a new branch here, we can merge it into this branch if we decide its worth special casing. I'd say it probably is, considering the code change is quite minimal change once we already have ForFilter defined.

BobTheBuidler · 2025-08-13T14:44:07Z

I don't yet think the failing docs workflow is related to this PR, given that I only added a single bullet point to the native_operations.rst, though can't say for sure

JukkaL · 2025-08-15T13:56:49Z

I wouldn't think to make a primitive for iteration solely over itertools.filterfalse, but considering it would share the same object and only require one more boolean attribute, I think it makes sense to support here. What do you guys think?

filterfalse seems to be used quite rarely (on the order of 100x less often than filter, based on a large codebase I have access to), so I think it's not worth supporting it at this time. My general guidance is to avoid itertools in any performance-sensitive code, so not having any support any of these makes it easier to teach users what to avoid. Otherwise users would have to remember which functions in itertools have optimized support.

JukkaL

Thanks for the PR, filter is a fairly common builtin function, so specializing it is useful.

JukkaL · 2025-08-15T13:58:46Z

mypyc/doc/native_operations.rst

@@ -54,3 +54,5 @@ These variants of statements have custom implementations:
 * ``for ... in seq:`` (for loop over a sequence)
 * ``for ... in enumerate(...):``
 * ``for ... in zip(...):``
+* ``for ... in filter(...):``
+* ``for ... in itertools.filterfalse(...):``


As mentioned in my comment, this seems rare enough that it doesn't seem worth it to support it. Every special case adds some overhead to users, as they may try to remember all the stdlib features with optimized primitives.

Is there currently any API for a "mypyc plugin" like a mypy plugin? Or future plans for one?

Could be cool to have a mypyc functools plugin and a mypyc itertools plugin, etc, to cover special casing of things that don't necessarily belong in the main mypy repo

something like iterools.accumulate would see quite large performance increases if we could handle the math operations in C. (this is something I wanted/needed for my own libs and was driving towards with these generator helpers but I'll just fork mypy for now to implement)

but to compile optimized C code for other fun things like functools.lru_cache and functools.cached_property would be really really cool (and fast) (and useful (in niche cases)) if there was a way it could be done without polluting the main repo

though I do think some functools special casing might still be appropriate in the main repo, if not now as a longer term goal. even though that isn't the current way things are done, it seems like we've already started in that direction (see the singledispatch feature set)

What if we just implement but don't document this, so as to add no overhead to users?

JukkaL · 2025-08-15T14:00:30Z

mypyc/irbuild/for_helpers.py

+            fake_call_expr = CallExpr(self.filter_func_def, [self.index], [ARG_POS], [None])
+
+            # I put this here to prevent a circular import
+            from mypyc.irbuild.expression import transform_call_expr


Similar to my comment in the map PR, you can use builder.accept to avoid the cyclic dependency.

Interesting! I commented this out and replaced it with builder.accept, and it broke the IR! Check out the test failures.

Once you've seen them I'll revert this to its original state so we're not boxing and unboxing unnecessarily. I'll add a TODO comment to debug the builder.accept issue and then replace this hacky import with builder.accept when its ready

JukkaL · 2025-08-15T14:02:20Z

mypyc/test-data/irbuild-basic.test

+L7:
+    return s
+
+[case testForFilterStr]


This seems specialized enough to move to a run test (see my comments in other PRs about our conventions related to irbuild tests).

JukkaL · 2025-08-15T14:03:25Z

mypyc/test-data/run-loops.test

+print(f([5, 15, 25]))
+print(f([]))
+
+[out]


Please merge all the tests under a single [case ...], leave out driver.py, use assertions instead of [out] and have test cases in def test_... functions. (See my similar comment in the map PR for the motivation.)

JukkaL · 2025-08-15T14:12:05Z

mypyc/doc/native_operations.rst

@@ -54,3 +54,5 @@ These variants of statements have custom implementations:
 * ``for ... in seq:`` (for loop over a sequence)
 * ``for ... in enumerate(...):``
 * ``for ... in zip(...):``
+* ``for ... in filter(...):``


Have you considered supporting list(filter(...)) as well -- this seems quite common (in a follow-up PR)?

Yep, I actually have that drafted already. but it won't be a special case for list(filter(...)) it will be a special case for [list|tuple|set](some_builtin_we_have_a_helper_for_in_for_helpers(...)) which will account for any builtin we have ForGenerator helpers for

Fwiw this was part of the intent behind the list-built-from-range tests

I wasn't actually testing that we can build a list from a range, I was preparing IR to reflect how this helper would change the C implementation. Will work for map, filter, range, zip, enumerate, and future ops with special-case gen helpers

BobTheBuidler · 2025-08-15T15:17:36Z

Thanks for the feedbacks. I have a few busy days ahead of me so I'll have to table these for now, but can try to get them all fixed up next week.

for more information, see https://pre-commit.ci

BobTheBuidler and others added 5 commits August 12, 2025 02:16

[mypyc] feat: ForFilter generator helper for builtins.filter

b3f3eab

[pre-commit.ci] auto fixes from pre-commit.com hooks

67818c6

for more information, see https://pre-commit.ci

fix: add filter to ir fixtures

74b7a6e

fix: run tests

eeb09ab

[pre-commit.ci] auto fixes from pre-commit.com hooks

ddc13b8

for more information, see https://pre-commit.ci

BobTheBuidler commented Aug 12, 2025

View reviewed changes

mypyc/irbuild/for_helpers.py Outdated Show resolved Hide resolved

BobTheBuidler and others added 3 commits August 12, 2025 03:58

test with None

fc12cea

Merge branch 'for-filter' of https://github.com/BobTheBuidler/mypy in…

5ce8148

…to for-filter

[pre-commit.ci] auto fixes from pre-commit.com hooks

54ad04e

for more information, see https://pre-commit.ci

BobTheBuidler and others added 13 commits August 12, 2025 15:06

IR cases for testing C calls

eae9209

feat: handle native calls and primitive ops

9941d54

Merge branch 'for-filter' of https://github.com/BobTheBuidler/mypy in…

71b27ef

…to for-filter

[pre-commit.ci] auto fixes from pre-commit.com hooks

5237f0b

for more information, see https://pre-commit.ci

Update run-loops.test

d68b833

Update for_helpers.py

c9680dc

test primitive op

5bf4b22

feat: use speciailizers

c39bb4a

Merge branch 'for-filter' of https://github.com/BobTheBuidler/mypy in…

8e43b2e

…to for-filter

Revert "Update for_helpers.py"

9dceb9a

This reverts commit c9680dc.

[pre-commit.ci] auto fixes from pre-commit.com hooks

7c8053f

for more information, see https://pre-commit.ci

add to docs

8aff832

Merge branch 'for-filter' of https://github.com/BobTheBuidler/mypy in…

0d2c019

…to for-filter

BobTheBuidler changed the title ~~[mypyc] feat: ForFilter generator helper for builtins.filter~~ [mypyc] feat: ForFilter generator helper for builtins.filter Aug 12, 2025

BobTheBuidler mentioned this pull request Aug 13, 2025

[mypyc] feat: extend ForFilter to support filterfalse BobTheBuidler/mypy#4

Open

Update for_helpers.py

cec1a5d

BobTheBuidler added 2 commits August 13, 2025 11:48

Merge branch 'master' into for-filter

5170a10

Merge branch 'master' into for-filter

ba5a978

BobTheBuidler added 2 commits August 14, 2025 03:00

Update native_operations.rst

572793c

Merge branch 'master' into for-filter

55ed2d6

JukkaL reviewed Aug 15, 2025

View reviewed changes

BobTheBuidler and others added 3 commits August 16, 2025 10:40

Update for_helpers.py

dbbbb57

[pre-commit.ci] auto fixes from pre-commit.com hooks

0bc1d26

for more information, see https://pre-commit.ci

Update for_helpers.py

7d56fa9

Uh oh!

[mypyc] feat: ForFilter generator helper for builtins.filter #19643

Are you sure you want to change the base?

[mypyc] feat: ForFilter generator helper for builtins.filter #19643

Uh oh!

Conversation

BobTheBuidler commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

BobTheBuidler commented Aug 12, 2025

Uh oh!

BobTheBuidler commented Aug 13, 2025

Uh oh!

BobTheBuidler commented Aug 13, 2025

Uh oh!

JukkaL commented Aug 15, 2025

Uh oh!

JukkaL left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BobTheBuidler Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BobTheBuidler Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BobTheBuidler Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BobTheBuidler commented Aug 15, 2025

Uh oh!

Uh oh!

[mypyc] feat: ForFilter generator helper for `builtins.filter` #19643

[mypyc] feat: ForFilter generator helper for `builtins.filter` #19643

BobTheBuidler commented Aug 12, 2025 •

edited

Loading

BobTheBuidler Aug 15, 2025 •

edited

Loading

BobTheBuidler Aug 15, 2025 •

edited

Loading

BobTheBuidler Aug 15, 2025 •

edited

Loading