| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
| else { | ||
| /* Sub-iter - calculate on the fly */ | ||
| VISIT(c, expr, gen->iter); | ||
| ADDOP(c, LOC(gen->iter), GET_AITER); |
There was a problem hiding this comment.
Hm, is this change necessary? I think the test passes without it.
Sorry, something went wrong.
There was a problem hiding this comment.
I think so. There was a bug in previous commit. I've reverted related logic in codegen.c.
Sorry, something went wrong.
|
I've reverted all generator expressions related changes in codegen.c. |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM.
Sorry, something went wrong.
|
@markshannon Do you want to take a look at this fix? Do you have time to do? |
Sorry, something went wrong.
|
@efimov-mikhail are you sure that this isn't adding back too many GET_ITER/GET_AITER instructions? We should have tests for generator expressions and list comprehensions, both sync and async that __iter__ is called only once. |
Sorry, something went wrong.
|
I've a had a quick look at the generated bytecode and looks like this PR is OK. @Yhg1s if you need this in ASAP, I think it's good to merge. |
Sorry, something went wrong.
There was a problem hiding this comment.
I'd like tests to check that __iter__/__aiter__ is only called once in inlined comprehensions, but that can wait for another PR.
Sorry, something went wrong.
Yes, changes in these two PRs are very similar. Although, there aren't exactly the same. But on 3.13 with #135390 being merged there is a different solution (from #125846). So, I have a question why we don't revert #125846? |
Sorry, something went wrong.
|
Was not it reverted as the result of all these changes? |
Sorry, something went wrong.
It was reverted here. |
Sorry, something went wrong.
|
Thank you, this is a great observation. |
Sorry, something went wrong.
I don't think I was aware of bytecode changes in 31.1. I tested the stdlib's compiled bytecode (including Lib/test) and saw no differences between 13.0, 13.3 and my rollback (and saw many changes between 13.3, 13.4 and 13.4 + the other fix-forward, but I didn't bother trying with this one applied). In any case it's too late to fix a change in bytecode that slipped into 13.1, if 13.1-13.3 are all the same we should stick with that. |
Sorry, something went wrong.
|
It is not only change in bytecode generation. It is a behavior change in comparison with 3.12, 3.13.0 and 3.13.4. |
Sorry, something went wrong.
|
To be precise, there is a bytecode change: -> % ./python
Python 3.13.0 (tags/v3.13.0:60403a5409, Jun 11 2025, 19:23:44) [GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import dis
>>> dis.dis('(x for x in range(10))')
0 RESUME 0
1 LOAD_CONST 0 (<code object <genexpr> at 0x7fa6c309f450, file "<dis>", line 1>)
MAKE_FUNCTION
LOAD_NAME 0 (range)
PUSH_NULL
LOAD_CONST 1 (10)
CALL 1
GET_ITER
CALL 0
RETURN_VALUE
Disassembly of <code object <genexpr> at 0x7fa6c309f450, file "<dis>", line 1>:
1 RETURN_GENERATOR
POP_TOP
L1: RESUME 0
LOAD_FAST 0 (.0)
L2: FOR_ITER 6 (to L3)
STORE_FAST_LOAD_FAST 17 (x, x)
YIELD_VALUE 0
RESUME 5
POP_TOP
JUMP_BACKWARD 8 (to L2)
L3: END_FOR
POP_TOP
RETURN_CONST 0 (None)
-- L4: CALL_INTRINSIC_1 3 (INTRINSIC_STOPITERATION_ERROR)
RERAISE 1
ExceptionTable:
L1 to L4 -> L4 [0] lastiand -> % ./python
Python 3.13.1 (tags/v3.13.1:0671451779, Jun 11 2025, 19:25:28) [GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import dis
>>> dis.dis('(x for x in range(10))')
0 RESUME 0
1 LOAD_CONST 0 (<code object <genexpr> at 0x7f4332397bc0, file "<dis>", line 1>)
MAKE_FUNCTION
LOAD_NAME 0 (range)
PUSH_NULL
LOAD_CONST 1 (10)
CALL 1
GET_ITER
CALL 0
RETURN_VALUE
Disassembly of <code object <genexpr> at 0x7f4332397bc0, file "<dis>", line 1>:
1 RETURN_GENERATOR
POP_TOP
L1: RESUME 0
LOAD_FAST 0 (.0)
GET_ITER
L2: FOR_ITER 6 (to L3)
STORE_FAST_LOAD_FAST 17 (x, x)
YIELD_VALUE 0
RESUME 5
POP_TOP
JUMP_BACKWARD 8 (to L2)
L3: END_FOR
POP_TOP
RETURN_CONST 0 (None)
-- L4: CALL_INTRINSIC_1 3 (INTRINSIC_STOPITERATION_ERROR)
RERAISE 1
ExceptionTable:
L1 to L4 -> L4 [0] lasti
>>> You can see additional GET_ITER instruction before FOR_ITER. IMO, it's better to revert this change and fix regression in #127682. |
Sorry, something went wrong.
|
I've created new PR for 3.13: #135403 |
Sorry, something went wrong.
There were 3 tests related to this topic, and I've added the fourth. |
Sorry, something went wrong.
|
We should remove backport-3.13 label. And what about changes on main branch? |
Sorry, something went wrong.
|
Thanks, this reverts the 3.14 async generator back to 3.12/3.13 behaviour: >>> (x async for x in None)
Traceback (most recent call last):
File "<python-input-1>", line 1, in <module>
(x async for x in None)
^^^^
TypeError: 'async for' requires an object with __aiter__ method, got NoneType
Yes, please do for main/3.15. |
Sorry, something went wrong.
|
I've started related d.p.o. thread here: https://discuss.python.org/t/generator-expressions-behavior-with-non-iterable-objects/96329. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This PR could be a part of #135225.
Changes in behavior of async generator expressions are also reverted.
@serhiy-storchaka @markshannon @Yhg1s