gh-151907: Avoid creating temporary objects in some comprehensions by ZeroIntensity · Pull Request #151908 · python/cpython

ZeroIntensity · 2026-06-22T06:46:47Z

The following code:

[i for i in range(42)]

is essentially turned into:

for i in range(42):
    pass

Issue: Avoid construction of objects in comprehensions without targets #151907

iritkatriel · 2026-06-22T07:30:07Z

I think this might change semantics for the dict case - if a key is not hashable.

ZeroIntensity · 2026-06-22T07:41:57Z

Yeah, good catch. This breaks set too:

{x for x in [[]]}  # No error!

I'll just remove set and dict for now, but it might still be possible to avoid object creation by calling PyObject_Hash. Not worth doing that here though.

This was a breaking change because hash() was no longer called on the elements.

iritkatriel · 2026-06-22T08:07:33Z

Yeah, good catch. This breaks set too:
{x for x in [[]]}  # No error!
I'll just remove set and dict for now, but it might still be possible to avoid object creation by calling PyObject_Hash. Not worth doing that here though.

Maybe also add tests for these cases.

sergey-miryanov · 2026-06-22T14:40:28Z

        ]
        self.codegen_test(snippet, expected)
+
+    def test_no_target_comp_optimization(self):


I believe you need to add a test where the loop body has a side effect. IIUC, the body with thread.join() should be turned into a method call, rather than just pass.

iritkatriel · 2026-06-23T19:29:51Z

The async case is not handled correctly here. This segfaults:

import asyncio
async def foo(XS):
    [x async for x in XS]

async def agen():
     yield 1
     yield 2
asyncio.run(foo(agen()))

Assertion failed: (PyList_Check(self)), function _PyList_AppendTakeRef, file pycore_list.h, line 40.
zsh: abort      ./python.exe

ZeroIntensity · 2026-06-23T20:28:04Z

Fixed.

Avoids an "env-changed" error with regrtest.

iritkatriel · 2026-06-30T15:00:57Z

+    if (value->kind == ListComp_kind) {
+        /* Optimization: Don't bother creating structures if they won't be
+         * used. */
+        return codegen_listcomp_impl(c, value, /*avoid_creation=*/true);


It might be nicer if we were not calling codegen_listcomp_impl here but rather setting a boolean on the compiler_unit struct to track whether we are in the topmost expression or not. What do you think?

Not opposed to the idea, but it feels slightly weird in cases where the thing being compiled isn't an expression. A compiler->has_no_target flag in a FunctionDef, for example, has no real meaning (because compiler->has_no_target == 0 would imply "has target", but that doesn't make sense for a statement). I do think it'll make future optimizations like this easier, though.

I was thinking of a flag like is_subexpression.

How would I implement that? I can't do like METADATA(c)->u_is_subexpression = 1 in codegen_stmt_expr because that would also affect listcomps inside the one we're optimizing (like [x for x in [y for y in z]]). I could add a stack or something like that, but that would significantly increase the complexity of this PR.

How about something like this:

static int codegen_visit_expr(compiler *c, expr_ty e) { if (Py_EnterRecursiveCall(" during compilation")) { return ERROR; } return codegen_visit_expr_impl(c, e, false); } static int codegen_visit_unused_expr(compiler *c, expr_ty e) { return codegen_visit_expr_impl(c, e, true); } static int codegen_visit_expr_impl(compiler *c, expr_ty e, bool result_is_unused) {

#define VISIT_UNUSED(C, TYPE, V) \ RETURN_IF_ERROR(codegen_visit_unused_ ## TYPE((C), (V)))

and then in codegen_stmt_expr use VISIT_UNUSED instead of VISIT?

Then codegen_stmt_expr doesn't need to do anything special for ListComp_kind.

I like that idea. I just pushed it, let me know what you think.

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>

iritkatriel · 2026-07-02T11:01:45Z

+                ADDOP_I(c, loc, SWAP, 2);
+            }
+        } else {
+            ADDOP_I(c, loc, COPY, 1);


can is_inlined be True here?

Yes, do I need to do anything special for it?

iritkatriel · 2026-07-02T11:06:17Z

+                VISIT(c, expr, elt);
+                ADDOP(c, elt_loc, POP_TOP);
+                break;
+            }


I think we need to apply VISIT to the starred case even when !avoid_creation, in case unpacking errors. No?

iritkatriel · 2026-07-02T16:45:08Z

        case COMP_LISTCOMP:
+            if (avoid_creation) {
+                if (elt->kind == Starred_kind) {
+                    RETURN_IF_ERROR(codegen_starred_comprehension(c, elt_loc, elt->v.Starred.value, /*discard=*/true));


I don't follow. Why don't we just do VISIT(c, expr, elt->v.Starred.value); like below for the !avoid_creation case?

elt->v.Starred.value only creates an iterator object, but we need to exhaust the whole thing in order to get all the side effects. It wasn't necessary before because LIST_EXTEND does the unpacking for us.

iritkatriel · 2026-07-02T17:27:42Z

 }

+static int
+codegen_starred_comprehension(compiler *c, location loc, expr_ty value, bool discard)


This name doesn't seem correct. This function is not actually about comprehensions, right?

ZeroIntensity added 4 commits June 22, 2026 01:36

Initial implementation.

b4a6d81

Apply the optimization to sets and dicts.

77251a2

Tests.

d80384a

Add blurb.

d2749ae

ZeroIntensity requested review from iritkatriel and markshannon as code owners June 22, 2026 06:46

bedevere-app Bot mentioned this pull request Jun 22, 2026

Avoid construction of objects in comprehensions without targets #151907

Open

bedevere-app Bot added the awaiting core review label Jun 22, 2026

iritkatriel reviewed Jun 22, 2026

View reviewed changes

Comment thread Python/codegen.c

Remove dict and set stuff.

cf1bb55

This was a breaking change because hash() was no longer called on the elements.

sergey-miryanov reviewed Jun 22, 2026

View reviewed changes

ZeroIntensity added 2 commits June 22, 2026 14:54

Add tests for hashing errors on dict and set comprehensions.

1988b90

Add a test ensuring that side effects are retained.

c010589

Fix async list comprehensions.

c1aa785

Don't use asyncio for testing async for.

a750ed5

Avoids an "env-changed" error with regrtest.

iritkatriel reviewed Jun 30, 2026

View reviewed changes

ZeroIntensity and others added 2 commits June 30, 2026 13:27

Update Lib/test/test_compiler_codegen.py

cdbcfad

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>

Use a more generic implementation.

5d9ccf3

iritkatriel reviewed Jul 2, 2026

View reviewed changes

Fix the optimization for starred comprehensions.

ad24c19

iritkatriel reviewed Jul 2, 2026

View reviewed changes

Use a better name.

9fe6718

Uh oh!

Uh oh!

Conversation

ZeroIntensity commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

iritkatriel commented Jun 22, 2026

Uh oh!

ZeroIntensity commented Jun 22, 2026

Uh oh!

iritkatriel commented Jun 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iritkatriel commented Jun 23, 2026

Uh oh!

ZeroIntensity commented Jun 23, 2026

Uh oh!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ZeroIntensity commented Jun 22, 2026 •

edited

Loading