Change YIELD/YIELD_FROM to do not increment opline#15328
Closed
arnaud-lb wants to merge 7 commits intophp:masterfrom
Closed
Change YIELD/YIELD_FROM to do not increment opline#15328arnaud-lb wants to merge 7 commits intophp:masterfrom
arnaud-lb wants to merge 7 commits intophp:masterfrom
Conversation
bwoebi
reviewed
Aug 10, 2024
Zend/zend_generators.c
Outdated
| * generator is being force-closed (opline points to the start of a finally | ||
| * block). | ||
| */ | ||
| if (EXPECTED(generator->execute_data->opline->opcode != ZEND_HANDLE_EXCEPTION |
Member
There was a problem hiding this comment.
The check for ZEND_HANDLE_EXCEPTION is not really necessary as EG(exception_op) has a couple ops at once. Only thing needing check is the force close.
Though, for that case, you can just decrement the opline in zend_generator_dtor_storage before calling this function, eliminating the need for the branch altogether.
Member
|
This changes the ReflectionGenerator::getExecutingLine() output, but I believe that's a good thing. |
Member
Author
|
This targets master, but do you think we can target 8.2? This may be a breaking change for extensions |
Member
|
No, you should not target 8.2 I think. I suspect you need to merge the other PR for 8.2, then use this one on master. |
* PHP-8.2: [ci skip] NEWS for phpGH-15275 Fix crash during GC of suspended generator delegate (php#15275)
* PHP-8.3: [ci skip] NEWS for phpGH-15275 [ci skip] NEWS for phpGH-15275 Fix crash during GC of suspended generator delegate (php#15275)
arnaud-lb
force-pushed
the
delegate-generator-exception-crash-2-master
branch
from
8a7dcbd to
84ea7a6
Compare
nicolas-grekas
added a commit
to symfony/symfony
that referenced
this pull request
Aug 12, 2024
…ndre-daubois) This PR was merged into the 5.4 branch. Discussion ---------- [VarDumper] Fix generator dump format for PHP 8.4 | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT Generator dump format likely changed because of php/php-src#15328. I propose to update the test dedicated to dumping generators in PHP >= 8.4. cc `@xabbuh` Commits ------- 6bb252d [VarDumper] Fix generator dump format for PHP 8.4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an alternative of #15275
YIELDandYIELD_FROMincrement opline before returning, but in most places we need the opline to point to theYIELDandYIELD_FROM.Here I change
YIELD/YIELD_FROMto not increment opline. This simplifies the code a fixes a few crashes.