Fixed bug #78434 #5344
Fixed bug #78434 #5344
Conversation
|
@bwoebi What do you think about this? |
| yield from $generator; | ||
|
|
||
| $generator = $function(); | ||
| // Comment out this line to get the correct result. |
There was a problem hiding this comment.
| // Comment out this line to get the correct result. |
| @@ -0,0 +1,28 @@ | |||
| --TEST-- | |||
| Bug #78434: Generator yields no items after valid() call | |||
There was a problem hiding this comment.
I was a bit wrong in the bug description. The generator just skips the first item. See https://3v4l.org/2HBSX
| Bug #78434: Generator yields no items after valid() call | |
| Bug #78434: Generator skips first item after valid() call |
|
Well, the intended semantics of i.e. if a same generator is yielded from different sources, they must always reflect the current iteration point of the generator and only next() shall be able to advance the generator. |
Zend/zend_generators.c
Outdated
| return; | ||
| if (UNEXPECTED(orig_generator->flags & ZEND_GENERATOR_DO_INIT)) { | ||
| orig_generator->flags &= ~ZEND_GENERATOR_DO_INIT; | ||
| if (!Z_ISUNDEF(generator->value)) { |
There was a problem hiding this comment.
This check is bogus … because the direct yielded generator might not be primed yet (i.e. not have a value), but the inner generator (i.e. see the test) may.
There was a problem hiding this comment.
I don't get what you mean. The value is only stored on the inner generator. This check matches what was there before as well.
|
Essentially your new proposed semantics are: advance the yield from'ed generator once unless it has not been primed yet, then prime it. Sounds weird to me. |
That's not the intention. This is supposed to work as you described in your previous comment (which does not match the current behavior). I've now pushed another commit to restore the behavior on |
|
I can't spot anything inherently wrong with this now. To be sure maybe run it once against a generator intensive test-suite like in amp once? |
|
@bwoebi Ran this against amp/http-client/http-server, no issues. As this is still a change in behavior that may have unexpected consequences, I'm only going to apply this to 7.4 though. |
|
Merged as 823a956 into 7.4 upwards. |
Fix for https://bugs.php.net/bug.php?id=78434. Set the
ZEND_GENERATOR_DO_INITflag whenever we perform ayield from, not when while performing "ensure initialized".The intention here is that we should take the
current()value of the generator before we advance it, unless the generator is before first yield, in which case we do need to advance it.I'm not really sure if the implementation is right, or even if the semantics are right... Especially not sure if the change in
Zend/tests/generators/gc_with_root_parent_mismatch.phptmakes sense.