On 05.08.2024 at 13:04, Derick Rethans wrote:

> Xdebug uses exit for exactly that too. For control flow analysis. And I
> also always have considered it to be a control flow instruction.
>
> I see no benefit in changing it to a function, especially because
> there will never be a function "exit" from it, just only an "entry".
> This breaks function execution symmetry (and causes issues with Xdebug
> when I last tried to make it work with a development branch for this
> RFC).
>
> As the RFC is scarce on mitigations for this, I am currently voting "no"
> as I am unsure how certain features in Xdebug could remain working. I
> have written to the list on other reasons before
> (https://externals.io/message/123277#123450) without a conclusion.
>
> I'll consider changing it to yes if there is a commitment for addressing
> these feature-maintaining-requirements to keep Xdebug working, either
> through new APIs (think observer) or other mitigations.

Hmm, so far I only had skimmed the RFC and the related discussions, but
now I checked out the suggested implementation[1].  Then I tried to
build with PECL/uopz, and of course that failed because ZEND_EXIT is no
longer there.  Okay, quickly drop that usage; build succeeds.  Then I ran

<?php
uopz_set_return("exit", function () {echo "hello";}, true);
exit;
?>

And got

hello

Previously, no output was echoed.  While that seems to be fixable in
uopz even without further changes to the implementation as mentioned by
Derick regarding Xdebug, I am concerned that changing `exit` to be a
proper function (although it isn't quite, since the parens could be
omitted) does more harm than good, at least in a minor PHP version.  I
would assume that there are more extensions overriding the ZEND_EXIT
handler, which would now need to override the function handler in the
best case, or be unfixable in the worst case.  Wrt. to such extensions,
the change might even delay the adoption of PHP 8.4.  Had an attempt
been made to assess the BC break regarding dropping the ZEND_EXIT
opcode?  From a quick GH search[2] quite some code may be affected.

[1] <https://github.com/php/php-src/pull/13483>
[2] <https://github.com/search?q=ZEND_EXIT&type=code>

Cheers,
Christoph

--
Currently listening to Judas Priest "Breaking your code"

Reply via email to