Fix GH-23301: nested "yield from" repeats a value after "yield from []"

[lazerg]

GH-15375's fix made the DO_INIT re-advance guard in zend_generator_resume() read the flag from the delegating generator rather than from orig_generator. That flag is set by zend_generator_yield_from() and only ever cleared on orig_generator, so on a middle generator it stays set for the rest of its life.

When such a middle generator then delegates to a non-generator iterable (yield from []), it still sits on a ZEND_YIELD_FROM opline, so it is picked as the delegator even though no new generator link was established, and its stale DO_INIT suppresses the resume. The value it yielded last is presented a second time. Twig hits this on every template, since doDisplay() always ends with yield from [];.

Only treat the generator as the delegator when it actually delegated to another generator (node.parent is set); otherwise keep orig_generator as before. The GH-15375 tests still pass.

Fixes GH-23301

[LamentXU123]

I'd say this is an extremely crafted case that basically don't happens read world. But code-wise, this looks good.
@arnaud-lb Could you please take a look at this?

[kocsismate]

@LamentXU123 no it's actually a widespread problem, twig doesn't work basically: #22640 (comment)

[iliaal]

Fix looks right to me. node.parent is the correct discriminator: zend_generator_yield_from() sets it alongside DO_INIT and is only reached for a generator operand, so it separates a live delegation from the stale flag a middle generator keeps afterwards.

The scope is wider than the title suggests, though. Diffing against a pre-22640 build, current 8.4 also repeats the value for yield from new ArrayIterator([]) and for a non-empty array tail, triples it in a four-level chain, and yields a duplicate key: k0=A,k1=B,k0=B where 8.3 gives k0=A,k1=B. Every one of those matches the pre-22640 output again with this patch, and gh15375 including its shared-primed section still passes.

One thing for whoever merges it: merging into current PHP-8.4 auto-merges NEWS and silently drops the GH-23301 line, so it needs adding back by hand.

[arnaud-lb]

The root cause appears to be that middle() is still flagged with ZEND_GENERATOR_DO_INIT after yield from inner(). @lazerg did you consider this direction?

[arnaud-lb]

Suggested change

	yield from [];
	yield from ["D"];

because yield from [] is optimized out by opcache otherwise

[lazerg]

@arnaud-lb Yes, considered it. DO_INIT only has one consumer, the delegator->flags check in zend_generator_resume around line 779, so clearing it on middle when its delegation to inner ends should land on the same behavior as gating which generator gets picked as delegator.

I patched zend_generator_update_current to clear ZEND_GENERATOR_DO_INIT alongside node.parent = NULL and reran the GH-23301 repro plus a few variants (yield from new ArrayIterator([]), a non-empty array tail, a 4-level chain). Output is byte-identical to the current fix in every case, and Zend/tests/generators still passes 161/161.

Reason I kept the node.parent check instead: node.parent gets reset to NULL in two spots in zend_generators.c, the exception branch and the normal branch of zend_generator_update_current, so clearing DO_INIT at the source means keeping both in sync, and any future site that drops a parent link without also clearing the flag reintroduces the bug. Gating on node.parent at the single read site avoids that, since it checks live state instead of relying on every staleness-introducing site being caught.

Both come down to the same idea: middle's DO_INIT is meaningless once it's no longer actively delegating, the only difference is whether we neutralize it at the read site or scrub it at write time. Happy to switch to the clear-at-source version if you'd rather have it there.

[arnaud-lb]

Reverting to delegator = orig_generator is not enough, as orig_generator may be flagged DO_INIT too. For example, this prints B twice:

function inner() {
    yield "B";
}

function middle($inner) {
    yield from $inner;
    yield from ["D"];
}

function outer($middle) {
    yield from $middle;
}

$inner = inner();
$middle = middle($inner);
$outer = outer($middle);

var_dump($outer->current()); // B

$middle->next();
var_dump($middle->current()); // B again, despite ->next()

[lazerg]

Confirmed, thanks. That case also fails on an 8.5.8 built before 0ccff76, so it predates the GH-15375 change, and the node.parent gate does not cover it.

Moved to your direction in a03afc6: delegator = generator goes back to what 0ccff76 wrote, and the delegator loses ZEND_GENERATOR_DO_INIT once the guard above has read it. Clearing the flag before that guard instead of after it breaks gh15375.phpt, so it is only cleared on the path that continues.

One thing worth a look. Zend/tests/generators/backtrace_multi_yield_from.phpt had the old behaviour recorded in its expectations: it calls $gen2->next() on a middle generator too, and the expected output carries int(1) twice for that reason. I updated it, since each next() now advances.

The GH-23301 test covers both cases now and uses yield from ["D"] for the tail, per your other note. Zend/tests passes in full, and a set of nested yield from scenarios gives output identical to 8.5.8 apart from the two cases this fixes.

[arnaud-lb]

I would go as far as clearing delegator only, and only when the flag is set:

diff --git a/Zend/zend_generators.c b/Zend/zend_generators.c
index 25669f8e54d..8d5f1f35a91 100644
--- a/Zend/zend_generators.c
+++ b/Zend/zend_generators.c
@@ -776,10 +776,12 @@ ZEND_API void zend_generator_resume(zend_generator *orig_generator) /* {{{ */
                return;
        }
 
-       if (UNEXPECTED((delegator->flags & ZEND_GENERATOR_DO_INIT) != 0 && !Z_ISUNDEF(generator->value))) {
-               /* We must not advance Generator if we yield from a Generator being currently run */
-               orig_generator->flags &= ~ZEND_GENERATOR_DO_INIT;
-               return;
+       if (UNEXPECTED((delegator->flags & ZEND_GENERATOR_DO_INIT) != 0)) {
+               delegator->flags &= ~ZEND_GENERATOR_DO_INIT;
+               if (UNEXPECTED(!Z_ISUNDEF(generator->value))) {
+                       /* We must not advance an already initialized delegate on first resumption */
+                       return;
+               }
        }
 
        if (EG(active_fiber)) {

Then, assuming that delegator and generator point to the actual delegator and delegatee, we shouldn't need to clear ZEND_GENERATOR_DO_INIT in any other place in this function (we can remove multiple orig_generator->flags &= ~ZEND_GENERATOR_DO_INIT) since we are always checking+clearing the flag of the delegator before advancing the delegatee.

@@ -819,7 +821,7 @@ ZEND_API void zend_generator_resume(zend_generator *orig_generator) /* {{{ */
                        EG(current_execute_data) = original_execute_data;
                        EG(jit_trace_num) = original_jit_trace_num;
 
-                       orig_generator->flags &= ~(ZEND_GENERATOR_DO_INIT | ZEND_GENERATOR_IN_FIBER);
+                       orig_generator->flags &= ~ZEND_GENERATOR_IN_FIBER;
                        generator->flags &= ~(ZEND_GENERATOR_CURRENTLY_RUNNING | ZEND_GENERATOR_IN_FIBER);
                        return;
                }
@@ -882,7 +884,6 @@ ZEND_API void zend_generator_resume(zend_generator *orig_generator) /* {{{ */
                } else {
                        generator = zend_generator_get_current(orig_generator);
                        zend_generator_throw_exception(generator, NULL);
-                       orig_generator->flags &= ~ZEND_GENERATOR_DO_INIT;
                        delegator = orig_generator;
                        goto try_again;
                }
@@ -900,7 +901,7 @@ ZEND_API void zend_generator_resume(zend_generator *orig_generator) /* {{{ */
                goto try_again;
        }
 
-       orig_generator->flags &= ~(ZEND_GENERATOR_DO_INIT | ZEND_GENERATOR_IN_FIBER);
+       orig_generator->flags &= ~ZEND_GENERATOR_IN_FIBER;
 }
 /* }}} */

WDYT?

[lazerg]

The second hunk is right, applied in e380159. On entry delegator == orig_generator, so the guard block always clears orig_generator's flag before anything advances. The three later clears can never see it set.

The first hunk does not work. It fails the "shared primed" part of Zend/tests/generators/gh15375.phpt:

$gen1 = counter();   // yields 1, 2
$gen1->valid();      // primed
$gen2 = outer($gen1);
$gen3 = outer($gen2);

$gen3->current();    // 1
$gen2->next();
$gen2->current();    // expected 1, got 2
$gen2->next();
$gen2->current();    // expected 2, got nothing

$gen3->current() reaches the guard with delegator == $gen2 and $gen1->value already set, so it returns without advancing. If we clear $gen2's flag there, $gen3's read spends $gen2's own first touch. The later $gen2->next() finds no flag, advances $gen1 to 2, and the 1 is never presented to $gen2.

The flag tracks one thing per generator: this delegator has not yet handed its delegate's current value to a caller. So it must be cleared for the generator that did the read, which is orig_generator, not for the delegator we checked it on. That is what the current code does: clear orig_generator on the path that returns, clear delegator on the path that goes on to advance. It is still check and clear before every advance. Only the target of the clear differs by which generator consumed the value.

I reverted the first hunk locally and kept the second. With only the second applied, Zend/tests/generators passes 161/161 and the full Zend/tests run has 0 failures. My 11 case nested yield from battery (empty, non empty and ArrayIterator tails, 4 level chain, send(), throw(), getReturn(), destruction mid chain, the GH-15375 and backtrace-multi cases, and your repro) gives output identical to a03afc6.

[arnaud-lb]

Isn't the shared primed test in gh15375.phpt mostly the same as Zend/tests/generators/backtrace_multi_yield_from.phpt?

I'm not sure anymore what is the expected behavior here, but my understanding is that ZEND_GENERATOR_DO_INIT should mean that we haven't consumed the current "yield from" delegate yet, so do not advance it if it's already initialized.

This is consistent with delegating from arrays/Iterators, with two differences:

• We rewind array/Iterators but not Generators
• The "not consumed yet" information it stored on the delegator instead of the delegatee because the same Generator instance is shared between multiple delegator, unlike arrays and iterators.

So ZEND_GENERATOR_DO_INIT is just a replacement for information that exists in array/Iterator but not in Generator.

Based on that assumption, if we have a yield from tree gen3->gen2->gen1 with gen1 already initialized and we fetch the current value of gen3, this should clear the "not consumed yet" flag of gen3 and gen2. Advancing either gen3 or gen2 should then advance gen1.

The new behavior of gh15375.phpt seems right to me.

cc @bwoebi

[lazerg]

Good catch, they are the same shape. backtrace_multi_yield_from.phpt builds gen1 = gen(), gen2 = from(gen1), gen3 = from(gen2), reads $gen3->current(), then calls $gen2->next(). The only difference is that the gh15375.phpt block primes gen1 with valid() first, and backtrace_multi does not.

That difference is exactly where the two now disagree, and it is my PR that split them. a03afc6 changed backtrace_multi so $gen2->next() advances gen1, which is your model. The shared primed block keeps the opposite for the primed case.

I measured the current branch on a 3 level chain with counter() yielding 1, 2, 3:

case	$gen3->current()	then	result
gen1 primed with valid()	1	$gen2->next(); $gen2->current()	1
gen1 primed with valid()	1	$gen3->next(); $gen3->current()	2
gen1 not primed	1	$gen2->next(); $gen2->current()	2
yield from [1,2,3] instead of gen1	1	$gen2->next(); $gen2->current()	2

So on the current branch only one case holds the value back: a primed Generator delegate, read through its direct delegator. Arrays behave the other way, and an unprimed Generator behaves the other way. In a 4 level chain the same split shows up: $gen3->next() advances, $gen2->next() does not.

With your first hunk applied all four rows give 2, and the 4 level chain agrees too. The whole Zend/tests run then has exactly one failure, the shared primed block in gh15375.phpt. My 11 case nested yield from battery is byte identical either way.

One more data point on provenance: gh15375.phpt including that shared primed block was added by 0ccff76 on 2026-07-08, so the expectation is a week old, not long standing behavior.

I have left e380159 as is and reverted the experiment locally. If you and @bwoebi settle on this model, I will apply your first hunk and update the shared primed expectation in the same commit. With counter() yielding 1 and 2 it becomes gen3 current: 1, gen2 current: 2, then an empty third line, which mirrors the trailing NULL in backtrace_multi_yield_from.phpt.

[bwoebi]

The new behaviour from the changed test looks correct to me.

Any access to the generator primes it. Any ->next() calls advances it. That's what the changed Zend/tests/generators/backtrace_multi_yield_from.phpt now properly asserts.

And yes, any next() call on an unconsumed generator is always immediately going to the second yield. (prime + next() basically)

And yes @arnaud-lb, priming the outer generator should recursively prime anything on it's chain, but not advance anything. (until next() is called)

[bwoebi]

Talking about gh-15375:

function outer(Generator $inner) {
    yield from $inner;
}

// A shared, pre-primed generator consumed through two nested "yield from"
// levels must still present its current value once to each consumer (the fix
// must not over-clear the middle level's first-touch).
echo "shared primed:\n";
function counter() {
    yield 1;
    yield 2;
}
$gen1 = counter();
$gen1->valid(); // $gen1 is at current = 1
$gen2 = outer($gen1);
$gen3 = outer($gen2);
echo "gen3 current: ", $gen3->current(), "\n"; // should be 1, current pos of $gen1
$gen2->next(); // $gen2 is advanced, advancing $gen1 too.
echo "gen2 current: ", $gen2->current(), "\n"; // should be 2
$gen2->next(); // advance again
echo "gen2 current: ", $gen2->current(), "\n"; // should be NULL

So Arnauds first hunk is what we want.

[lazerg]

Thanks both. Applied in b1f74a6, which is @arnaud-lb's first hunk unchanged.

On the NULL in your trace: there was no disagreement, I just read my own output wrong earlier. The block used echo, and echo NULL prints an empty string. Running your script gives 1, 2, then a blank line, and var_dump confirms the third value is NULL. So the trace matched what you wrote.

I switched that block from echo to var_dump so the NULL is visible instead of an empty line, which also matches how backtrace_multi_yield_from.phpt asserts the same shape. New expectation is int(1), int(2), NULL. I reworded the comment above it too, since it described the behaviour we are replacing.

The model now holds everywhere. Measured on a 3 level chain after reading $gen3->current():

delegate	$gen2->next(); $gen2->current()	before	after
primed Generator		1	2
unprimed Generator		2	2
yield from [1,2,3]		2	2

A 4 level chain agrees, and so does $gen3->next() in both. Before this commit only one case held its value back: a primed Generator read through its direct delegator. Arrays and unprimed Generators already advanced.

Zend/tests/generators passes 161/161 and the full Zend/tests run has 0 failures. backtrace_multi_yield_from.phpt passes as currently written, no further change needed there. My 11 case nested yield from battery is byte identical to the previous commit. I also checked the updated gh15375.phpt fails without this hunk and passes with it.