Repository navigation
Conversation
|
Thanks. Looks good to me. |
iliaal
left a comment
iliaal
left a comment
There was a problem hiding this comment.
Please rebase onto the current PHP-8.4 branch to resolve the merge conflict in ext/standard/var.c. The nested-export failure handling already landed in afe5635, including UNEXPECTED(result == FAILURE). Retain that check when resolving the overlap; the lazy-initialization exception check and regression tests remain useful.
| } | ||
| ZEND_GUARD_OR_GC_PROTECT_RECURSION(guard, EXPORT, zobj); | ||
| myht = zend_get_properties_for(struc, ZEND_PROP_PURPOSE_VAR_EXPORT); | ||
| if (EG(exception)) { |
There was a problem hiding this comment.
Consider using if (UNEXPECTED(EG(exception))) here to mark the exceptional exit. This is an optional branch-prediction hint; no performance improvement has been measured.
| if (val == &tmp) { | ||
| zval_ptr_dtor(val); | ||
| } | ||
| if (result == FAILURE) { |
There was a problem hiding this comment.
Consider using if (UNEXPECTED(result == FAILURE)) for this exceptional exit. Current PHP-8.4 already contains this failure handling with that wrapper, so please retain the upstream version when resolving the rebase conflict.
8ebc153 to
41ff2db
Compare
lazerg
changed the title
|
Rebased onto PHP-8.4 in 41ff2db. I kept the upstream failure check and added UNEXPECTED() to the lazy init check. I also dropped the NEWS change. Both tests are still there: gh24743_1 covers the hook case from the issue, and gh24743_2 fails without the lazy init check. |
var_export() printed partial output when a lazy object's initializer threw. The exception comes from zend_get_properties_for(), and php_var_export_ex() kept exporting after it. Now it releases the recursion guard and the properties and returns FAILURE.
The nested-object part (a property hook throwing inside a nested object) is already fixed on PHP-8.4 by afe5635. gh24743_1.phpt keeps that case covered, and gh24743_2.phpt covers the lazy object case.
Fixes #24743