gh-125038: redundant GET_ITER instructions are removed from genexpr code

[efimov-mikhail]

Adds CHECK_ITERABLE instruction. It checks that TOS is iterable or async iterable.
IMO, new simple instruction is a good way to prevent behavior changes.

Redundant GET_ITER instructions are removed from generator expression code.
Those were added during previous fix.

• Issue: SIGSEGV with generators after direct change in gi_frame #125038

---

📚 Documentation preview 📚: https://cpython-previews--126408.org.readthedocs.build/

[efimov-mikhail]

cc @markshannon, @JelleZijlstra

[ZeroIntensity]

I'll do the initial review on this later today. In the meantime, you can rebase against main to fix the JIT failures (see gh-126465).

[ZeroIntensity]

For future reference, don't force push. Everything is squashed onto main anyway.

[efimov-mikhail]

For future reference, don't force push. Everything is squashed onto main anyway.

I've used the Github button "rebase", it made force push.
But local merging with main and pushing to this branch is preffered, isn't it?

[JelleZijlstra]

I like to use the "Update branch" button, which merges in main without a need for local actions. We try to avoid force-pushes since they make it harder to track the history of the PR.

[ZeroIntensity]

In general, this is looking pretty good! Several comments, though, as this is a pretty big PR.

[ZeroIntensity]

Thank you! I think this is ready for a core dev to review it. @JelleZijlstra, would you mind taking a look?

[markshannon]

I thought the idea was to entirely remove the GET_ITER, now that the generator expression handles the conversion from iterable to iterator.

I realize that would be a slight change, but it makes generator expressions behave the same as generators:

>>> def make_gen(x):
...     for i in x:
...         yield i
...
>>> gen = make_gen(1)  # No error here
>>> next(gen)
Traceback (most recent call last):
  File "<python-input-28>", line 1, in <module>
    next(gen)
    ~~~~^^^^^
  File "<python-input-26>", line 2, in make_gen
    for i in x:
             ^
TypeError: 'int' object is not iterable

Creating a generator with a non-iterable is fine. It is only we try to iterate over it, that an error occurs.

[efimov-mikhail]

It's not possible to check arguments of an arbitrary generators, since it can be of any type.
For example, this generator is correct:

def make_range_gen(n, m):
    for i in range(n + m * 5):
        yield i

But generator expressions can have only one parameter, which should be Iterable.
So we can guarantee that argument is really Iterable.

Let's look at the solution with CHECK_ITERABLE bytecode.
Pros:

1. Backward compatibility in all details.
2. Early detection of incorrect code in genexprs.
3. Very small impact on perfomance, almost zero, since implementation of CHECK_ITERABLE is very simple.

Cons:

1. Small impact on memory size of compiled object (+2 bytes on each creation of genexpr).
2. New bytecode with little purpose, which uses one of byte values in range(256), and we want to keep those values for important bytecodes.

I'm not sure about the balance of this pros and cons for the typical user of Python.
Backward compatibility seems to be essential advantage.
But, of course, I can remove CHECK_ITERABLE, if you sure that solution without it is better.

@ZeroIntensity, @JelleZijlstra, what do you think?

[sobolevn]

Creating a generator with a non-iterable is fine. It is only we try to iterate over it, that an error occurs.

Can we please discuss this a bit more? I don't think that changing where the error happens is safe in terms of backward compatibility.

I am 100% sure that people expect error to happen when the gen is created and I am also sure that the code reflects that (validation, error handling, etc).

If we are going to break this - we need to issue a deprecation period.

I also don't fully understand why we are doing this.

Sorry, there's enough contention here that I'd like to have my check removed from the PR (it was only supposed to denote "ready for core review," anyway).