gh-151907: Avoid creating temporary objects in some comprehensions

[ZeroIntensity]

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]

Can we instead just wrap the LIST_APPEND in if !avoid_creation?

[ZeroIntensity]

I think we'd also need to wrap the LIST_EXTEND with that too; I opted for this approach to avoid duplicating that logic.

But do we need the separate VISIT path for the Starred case at all? I might be able to refactor it to look like this:

ISIT(c, expr, elt);
if (elt->kind == Starred_kind) {
    op = LIST_EXTEND;
}
else {
    op = LIST_APPEND;
}
if (!avoid_creation) {
    ADDOP_I(c, elt_loc, op, depth + 1);
}

[iritkatriel]

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

[ZeroIntensity]

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.

[iritkatriel]

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]

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]

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]

Fixed.

[iritkatriel]

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?

[ZeroIntensity]

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.

[iritkatriel]

I was thinking of a flag like is_subexpression.

[ZeroIntensity]

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.