Hello Internals,
I have just opened the vote for the "Transform exit() from a language construct into a standard function" RFC:
https://wiki.php.net/rfc/exit-as-function
The vote will last for two weeks until the 13th of August 2024.
Best regards,
Gina P. Banyard
Hi Gina!
On 30.07.2024 at 11:49, Gina P. Banyard wrote:
I have just opened the vote for the "Transform exit() from a language construct into a standard function" RFC:
PHP: rfc:exit-as-functionThe vote will last for two weeks until the 13th of August 2024.
As userland PHP developer, I always regarded `exit` as a control flow
instruction (quite similar to `break`), and as such I'm not really in
favor of converting it to a proper function (especially since it is not,
because the parantheses could be omitted).
Would that RFC imply that I would need to write `\exit` or have a `use
exit` clause to avoid dynamic namespace lookup? If so, I would be even
less in favor of that change.
I do understand your point about the type juggling semantics, but I
might have addressed that differently. I almost always use `exit`
without argument (and if, only with an int), and `die` always with a
string (and only for quick experiments), and I figure that this might be
what most contemporary code does (at least, I hope that the
`do_something() or die()` times have long gone). As such, having `exit`
and `die` as alias could be changed, sticking with `exit` as a control
flow instruction, and having `die` as proper function (which could even
be implemented in userland), where exit would allow an optional int
argument (like `break`), and die() a required string argument. Of
course, this would be a much bigger BC break, but it seems to me the
cleaner solution.
Cheers,
Christoph
On Tuesday, 30 July 2024 at 13:47, Christoph M. Becker <cmbecker69@gmx.de> wrote:
Hi Gina!
On 30.07.2024 at 11:49, Gina P. Banyard wrote:
> I have just opened the vote for the "Transform exit() from a language construct into a standard function" RFC:
> PHP: rfc:exit-as-function
>
> The vote will last for two weeks until the 13th of August 2024.As userland PHP developer, I always regarded `exit` as a control flow
instruction (quite similar to `break`), and as such I'm not really in
favor of converting it to a proper function (especially since it is not,
because the parantheses could be omitted).
You are not the first person to mention this.
I don't have any plans on doing anything else to exit, but I never really considered it to be a control flow instruction, but that is maybe just me.
Would that RFC imply that I would need to write `\\exit` or have a `use exit` clause to avoid dynamic namespace lookup? If so, I would be even
less in favor of that change.
No, since the new implementation where the token is preserved this is not needed.
You cannot define the function in a namespace, nor disable it, nor use it as a goto label.
The one benefit of having it become a proper function which can also be used as a statement is:
- Using named arguments
- Passing it to callable parameters
- Usual type juggling semantics
I do understand your point about the type juggling semantics, but I
might have addressed that differently. I almost always use `exit`
without argument (and if, only with an int), and `die` always with a
string (and only for quick experiments), and I figure that this might be
what most contemporary code does (at least, I hope that the
`do_something() or die()` times have long gone). As such, having `exit`
and `die` as alias could be changed, sticking with `exit` as a control
flow instruction, and having `die` as proper function (which could even
be implemented in userland), where exit would allow an optional int
argument (like `break`), and die() a required string argument. Of
course, this would be a much bigger BC break, but it seems to me the
cleaner solution.
This might be a good idea in the long term to properly split exit and die and have them take only integers and string respectively,
but I am not going to personally bother with this, even if I think this is a good idea.
Best regards,
Gina P. Banyard
On 30.07.2024 at 14:30, Gina P. Banyard wrote:
On Tuesday, 30 July 2024 at 13:47, Christoph M. Becker <cmbecker69@gmx.de> wrote:
Would that RFC imply that I would need to write `\\exit` or have a `use exit` clause to avoid dynamic namespace lookup? If so, I would be even
less in favor of that change.No, since the new implementation where the token is preserved this is not needed.
You cannot define the function in a namespace, nor disable it, nor use it as a goto label.
Okay, thanks for the clarification.
The one benefit of having it become a proper function which can also be used as a statement is:
- Using named arguments
- Passing it to callable parameters
- Usual type juggling semantics
Personally, I'm not interested in doing the first two points.
I do understand your point about the type juggling semantics, but I
might have addressed that differently. I almost always use `exit`
without argument (and if, only with an int), and `die` always with a
string (and only for quick experiments), and I figure that this might be
what most contemporary code does (at least, I hope that the
`do_something() or die()` times have long gone). As such, having `exit`
and `die` as alias could be changed, sticking with `exit` as a control
flow instruction, and having `die` as proper function (which could even
be implemented in userland), where exit would allow an optional int
argument (like `break`), and die() a required string argument. Of
course, this would be a much bigger BC break, but it seems to me the
cleaner solution.This might be a good idea in the long term to properly split exit and die and have them take only integers and string respectively,
but I am not going to personally bother with this, even if I think this is a good idea.
Fair enough.
Given that I'm very late to this party, and that I'm not strongly
opposed to the suggested change, I will refrain from voting.
Cheers,
Christoph
On Tue, 30 Jul 2024, Christoph M. Becker wrote:
On 30.07.2024 at 11:49, Gina P. Banyard wrote:
> I have just opened the vote for the "Transform exit() from a
> language construct into a standard function" RFC:
> PHP: rfc:exit-as-function
>
> The vote will last for two weeks until the 13th of August 2024.As userland PHP developer, I always regarded `exit` as a control flow
instruction (quite similar to `break`), and as such I'm not really in
favor of converting it to a proper function (especially since it is
not, because the parantheses could be omitted).
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
([RFC] Transform exit() from a language construct into a standard function - Externals) 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.
cheers,
Derick
--
https://derickrethans.nl | https://xdebug.org | https://dram.io
Author of Xdebug. Like it? Consider supporting me: Xdebug: Support
mastodon: @derickr@phpc.social @xdebug@phpc.social
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
([RFC] Transform exit() from a language construct into a standard function - Externals) 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] <Code search results · GitHub;
Cheers,
Christoph
--
Currently listening to Judas Priest "Breaking your code"
On Mon, Aug 5, 2024, at 6:04 AM, Derick Rethans wrote:
On Tue, 30 Jul 2024, Christoph M. Becker wrote:
On 30.07.2024 at 11:49, Gina P. Banyard wrote:
> I have just opened the vote for the "Transform exit() from a
> language construct into a standard function" RFC:
> PHP: rfc:exit-as-function
>
> The vote will last for two weeks until the 13th of August 2024.As userland PHP developer, I always regarded `exit` as a control flow
instruction (quite similar to `break`), and as such I'm not really in
favor of converting it to a proper function (especially since it is
not, because the parantheses could be omitted).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
([RFC] Transform exit() from a language construct into a standard function - Externals) 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.cheers,
Derick
While I support language and engine cleanup, if this change causes issues for Xdebug that is a fairly significant problem. For that reason I have shifted my Yes to a No for now. Like Derick, I will switch it back to a Yes should the Xdebug issue be resolved to his satisfaction. But "keep Xdebug working" is a rather mission-critical requirement for any RFC, as a practical matter.
--Larry Garfield
On Tue, Jul 30, 2024 at 3:52 AM Gina P. Banyard <internals@gpb.moe> wrote:
Hello Internals,
I have just opened the vote for the "Transform exit() from a language construct into a standard function" RFC:
PHP: rfc:exit-as-functionThe vote will last for two weeks until the 13th of August 2024.
Best regards,
Gina P. Banyard
After thinking about this several times over the course of discussion
and again now that it's in voting, I have decided to vote no. I am in
favor of this change, I just think given the concerns it's best to
wait for PHP 9.0 to do it. Maybe the concerns with control flow can be
improved by better inference/marking of functions which are
`noreturn`, at least for known functions (of which `\exit` definitely
would be).
Hi
On 8/5/24 13:04, Derick Rethans wrote:
As userland PHP developer, I always regarded `exit` as a control flow
instruction (quite similar to `break`), and as such I'm not really in
favor of converting it to a proper function (especially since it is
not, because the parantheses could be omitted).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).
This is false. The observers are perfectly capable of detecting that the `exit()` function returns / throws when observing `zend_observer_fcall_end_handler` as demonstrated by the following script:
<?php
class MyClass {
public function __destruct() {
echo __METHOD__, PHP_EOL;
}
}
function a() {
$dummy = new MyClass();
exit();
}
a();
Running this script as:
sapi/cli/php -d zend_test.observer.enabled=1 -d zend_test.observer.observe_all=1 test.php
will return the following output:
<!-- init 'php-src/test.php' -->
<file 'php-src/test.php'>
<!-- init a() -->
<a>
<!-- init exit() -->
<exit>
<!-- Exception: UnwindExit -->
</exit>
<!-- Exception: UnwindExit -->
</a>
<!-- init MyClass::__destruct() -->
<MyClass::__destruct>
MyClass::__destruct
</MyClass::__destruct>
<!-- Exception: UnwindExit -->
</file 'php-src/test.php'>
Leaving the exit() function will be observed as indicated by `</exit>`. It also shows that any destructors will be called and it also shows the internal UnwindExit exception.
Thus you are able to detect if `exit()` has successfully been called by observing an fcall_end and checking for `zend_is_unwind_exit(EG(exception))` to determine if the exception you're dealing with is the unwind exception.
That way you will also correctly handle that `exit()` is not successful, e.g. when a non-stringable class is passed to it and a TypeError is thrown, because then the output will look something like this:
<!-- init 'php-src/test.php' -->
<file 'php-src/test.php'>
<!-- init a() -->
<a>
<!-- init exit() -->
<exit>
<!-- Exception: TypeError -->
</exit>
<!-- Exception: TypeError -->
</a>
<!-- Exception: TypeError -->
</file 'php-src/test.php'>
<!-- init Error::__toString() -->
<Error::__toString>
<!-- init Error::getTraceAsString() -->
<Error::getTraceAsString>
</Error::getTraceAsString>
</Error::__toString>
Fatal error: Uncaught TypeError: exit(): Argument #1 ($code) must be of type string|int, MyClass given in php-src/test.php:11
Stack trace:
#0 php-src/test.php(11): exit(Object(MyClass))
#1 php-src/test.php(14): a()
#2 {main}
thrown in php-src/test.php on line 11
<!-- init MyClass::__destruct() -->
<MyClass::__destruct>
MyClass::__destruct
</MyClass::__destruct>
Best regards
Tim Düsterhus
Hi
On 8/5/24 14:52, Christoph M. Becker wrote:
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
Frankly from a userland developer PoV this looks entirely correct: If I override the `exit()` function, then I expect the `exit()` function to be overridden.
Best regards
Tim Düsterhus
Hi
On 8/5/24 18:06, Levi Morrison wrote:
After thinking about this several times over the course of discussion
and again now that it's in voting, I have decided to vote no. I am in
favor of this change, I just think given the concerns it's best to
wait for PHP 9.0 to do it. Maybe the concerns with control flow can be
improved by better inference/marking of functions which are
`noreturn`, at least for known functions (of which `\exit` definitely
would be).
As I've just explained in my response to Derick within the discussion thread related to this RFC: The information is already there, it just needs to be used.
For a test I've just added the following code snippet to `zend_compile_call()`:
zend_type return_type = fbc->common.arg_info[-1].type;
if (ZEND_TYPE_CONTAINS_CODE(return_type, IS_NEVER)) {
printf("%s has return type never\n", ZSTR_VAL(lcname));
} else {
printf("%s doesn't have return type never\n", ZSTR_VAL(lcname));
}
and then I executed the following test script:
<?php
namespace Foo;
function a() {
\strrev('foo');
exit(new \stdClass());
}
try {
a();
} catch (\Throwable) {}
echo "executed", PHP_EOL;
when running this script, I receive the following output:
strrev doesn't have return type never
exit has return type never
executed
Best regards
Tim Düsterhus
On 05.08.2024 at 18:52, Tim Düsterhus wrote:
On 8/5/24 14:52, Christoph M. Becker wrote:
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
Frankly from a userland developer PoV this looks entirely correct: If I
override the `exit()` function, then I expect the `exit()` function to
be overridden.
I should have clarified, that "fixing" means to restore the expected
behavior of uopz, namely that accidential attempts to override exit with
`uopz_set_return()` were silently ignored, but unless `uopz.exit=1` is
set, or `uopz_allow_exit(true)` is called, exit is ignored. Especially
the latter may be relied upon by tests for legacy code.
Christoph
On Monday, 5 August 2024 at 12:04, Derick Rethans <derick@php.net> wrote:
On Tue, 30 Jul 2024, Christoph M. Becker wrote:
> On 30.07.2024 at 11:49, Gina P. Banyard wrote:
>
> > I have just opened the vote for the "Transform exit() from a
> > language construct into a standard function" RFC:
> > PHP: rfc:exit-as-function
> >
> > The vote will last for two weeks until the 13th of August 2024.
>
> As userland PHP developer, I always regarded `exit` as a control flow
> instruction (quite similar to `break`), and as such I'm not really in
> favor of converting it to a proper function (especially since it is
> not, because the parantheses could be omitted).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
([RFC] Transform exit() from a language construct into a standard function - Externals) 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.cheers,
Derick--
https://derickrethans.nl | https://xdebug.org | https://dram.ioAuthor of Xdebug. Like it? Consider supporting me: Xdebug: Support
mastodon: @derickr@phpc.social @xdebug@phpc.social
I still do not comprehend your issue.
You can either do stuff at compile time, which has been explained before, or use the function call observer for anything runtime related.
The behaviour of exit() is completely observable with the current observer framework, so I don't see why anything else needs to be done.
Best regards,
Gina P. Banyard
On Monday, 5 August 2024 at 18:30, Christoph M. Becker <cmbecker69@gmx.de> wrote:
On 05.08.2024 at 18:52, Tim Düsterhus wrote:
> On 8/5/24 14:52, Christoph M. Becker wrote:
>
> > 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
>
> Frankly from a userland developer PoV this looks entirely correct: If I
> override the `exit()` function, then I expect the `exit()` function to
> be overridden.I should have clarified, that "fixing" means to restore the expected
behavior of uopz, namely that accidential attempts to override exit with
`uopz_set_return()` were silently ignored, but unless `uopz.exit=1` is
set, or `uopz_allow_exit(true)` is called, exit is ignored. Especially
the latter may be relied upon by tests for legacy code.Christoph
This sounds like a uopz extension issue that is easily fixed.
I am not sure why we should bend over backwards for extensions that allow to break usual userland semantics while preventing userland behaviour to be better.
Best regards,
Gina P. Banyard
On 05.08.2024 at 21:37, Gina P. Banyard wrote:
This sounds like a uopz extension issue that is easily fixed.
Most likely, yes, although still somebody has to provide a fix, and
someone has to do a new release.
I am not sure why we should bend over backwards for extensions that allow to break usual userland semantics while preventing userland behaviour to be better.
We shouldn't do one or the other without having weighted the pros and
cons. So what are the pros regarding improved userland behavior:
* we can use named arguments
So we can now write `exit(status = 1)` or `exit(status = "some error
message`). I don't see any improvement using named arguments for a
single argument function (maybe unless it is a boolean argument). And
for a string argument, I don't think that `status` is a sensible
parameter name here.
* pass to functions as callable
That might be a nice feature, but at least I have never seen the need to
pass `exit` as callable. And it wouldn't be hard to define a wrapper
function (e.g. `my_exit()`) if someone really needs to pass it as a
callable.
* does not respect strict_types and does not follow usual type juggling
semantics
That *might* be an issue (although I've never stumbled upon that), but
it seems to me that this could even be handled in the ZEND_EXIT handler.
Perhaps I've missed some pros, but it seems these have been the ones the
RFC explicitly mentions.
So, what are the cons:
* removing the ZEND_EXIT opcode
That immediately breaks any extension using it. A quick Github search
lists 3.1 k occurrences[1].
* making exit a "proper" function
It still can be called without parentheses, so I don't regard it as a
proper function. It *might* even be confusing to readers ("oh, now I
can call a function without arguments by omitting the parentheses – but
why doesn't it work for other functions?"). And the PHP manual even
states (emphasis mine):
| exit is a *language* *construct* and it can be called without
| parentheses if no status is passed.
Weighting the pros and cons is left as an exercise to the reader.
[1] <Code search results · GitHub;
Thanks,
Christoph
Am 30.07.2024, 11:49:52 schrieb Gina P. Banyard internals@gpb.moe:
Hello Internals,
I have just opened the vote for the “Transform exit() from a language construct into a standard function” RFC:
https://wiki.php.net/rfc/exit-as-functionThe vote will last for two weeks until the 13th of August 2024.
Best regards,
Gina P. Banyard
Hi Gina,
I was ambivalent to this before, but after reading the discussion, voted no for two reasons:
I also have a question: From a userland perspective, it looks it creates inconsistency calling exit with global namespace prefix, depending on parenthesis or? \exit(); will now work, but \exit; will not?
Also if https://github.com/php/php-src/pull/4084 taught me anything, trying to add eval support for disabled_functions, then maybe we should throw warnings instead when using that ini setting to disable op codes, instead of silently ignoring that this won’t work.
Am 06.08.2024, 09:43:21 schrieb Benjamin Außenhofer <kontakt@beberlei.de>:
Am 30.07.2024, 11:49:52 schrieb Gina P. Banyard internals@gpb.moe:
Hello Internals,
I have just opened the vote for the “Transform exit() from a language construct into a standard function” RFC:
https://wiki.php.net/rfc/exit-as-functionThe vote will last for two weeks until the 13th of August 2024.
Best regards,
Gina P. Banyard
Hi Gina,
I was ambivalent to this before, but after reading the discussion, voted no for two reasons:
- exit without parenthesis s still not a function, that does not makes handling this on extension level (tools, profilers) more reliable, instead more confusing. I don’t mind the work this might cause for an extension, so i don’t join Christoph’s and Derick’s argument here that this a break for extensions that disqualifies the change.
Tim pointed out to me that my argument here is wrong, since exit; will be transformed to exit() at compile time, I did not read this from my first read-through of the RFC, but now I see it in the text. So this point does not hold anymore.
- If it still does not allow me to set exit as disabled_functions, then this creates an inconsistency
That left me wondering why disabled_functions does not work, and I see the PR adds a special case for both functions. So disabling them „could“ potentially work.
I also have a question: From a userland perspective, it looks it creates inconsistency calling exit with global namespace prefix, depending on parenthesis or? \exit(); will now work, but \exit; will not?
Also if https://github.com/php/php-src/pull/4084 taught me anything, trying to add eval support for disabled_functions, then maybe we should throw warnings instead when using that ini setting to disable op codes, instead of silently ignoring that this won’t work.
On Tuesday, 6 August 2024 at 09:27, Benjamin Außenhofer <kontakt@beberlei.de> wrote:
Am 06.08.2024, 09:43:21 schrieb Benjamin Außenhofer <kontakt@beberlei.de>:
- If it still does not allow me to set exit as disabled_functions, then this creates an inconsistency
That left me wondering why disabled_functions does not work, and I see the PR adds a special case for both functions. So disabling them „could“ potentially work.
Indeed, as it would be a true function and disabling it would work as expected, as initially I did permit disabling it but I decided that might be a bit too weird for people.
In fact one could even redefine it after disabing it (like any other internal function).
It might be a bit harder to support redefining it after it has been disabled now that the implementation is at the parser level (as various people prefered).
In any case, ant technical issue that eval() was having are non-existing here.
Best regards,
Gina P. Banyard
On Monday, 5 August 2024 at 23:55, Christoph M. Becker <cmbecker69@gmx.de> wrote:
On 05.08.2024 at 21:37, Gina P. Banyard wrote:
> This sounds like a uopz extension issue that is easily fixed.
Most likely, yes, although still somebody has to provide a fix, and
someone has to do a new release.
Why should I investigate time to fix extensions when this RFC might not even land in the first place?
We do countless engine breaking changes without RFCs and without notices to extension maintainers other than UPGRADING.INTERNALS.
May I remind everyone that PHP 8.1 had a huge breaking change for extensions in utterly rewriting the stream API:
Zend Stream API has been changed to use "zend_string*" instead of "char*" [1]
Extension developers have somehow coped with it and/or just suffered in silence.
We didn't provide ANY help for extension developers to make the move easier.
It is getting frustrating that on one hand we treat extension developers like they are idiots incapable of updating their extension for minor changes in engine APIs and behaviour (or even just including the proper headers circling, back to *that* whole drama)
While at the same time kicking them in the face with massive, and potentially clunky #ifdef checks, code churn for major engine API changes that get ZERO discussion and expect them to be competent at their job.
Could we maybe start respecting the intelligence of extension developers?
And if not, can we stop pretending that we care about them when we, as a project, clearly don't seem to care in our actual action?
Because I find it a tad strange that every time I point the above stream API design change, the only thing I hear back is crickets.
> I am not sure why we should bend over backwards for extensions that allow to break usual userland semantics while preventing userland behaviour to be better.
We shouldn't do one or the other without having weighted the pros and
cons. So what are the pros regarding improved userland behavior:* we can use named arguments
So we can now write `exit(status = 1)` or `exit(status = "some error message`). I don't see any improvement using named arguments for a
single argument function (maybe unless it is a boolean argument). And
for a string argument, I don't think that `status` is a sensible
parameter name here.
Please propose a better name then.
* pass to functions as callable
That might be a nice feature, but at least I have never seen the need to
pass `exit` as callable. And it wouldn't be hard to define a wrapper
function (e.g. `my_exit()`) if someone really needs to pass it as a
callable.
The fact that one might need to do this workaround is bad already, and not a justification against this RFC.
* does not respect strict_types and does not follow usual type juggling
semanticsThat might be an issue (although I've never stumbled upon that), but
it seems to me that this could even be handled in the ZEND_EXIT handler.
Now see this is strange to me, as I clearly remember hitting this issue while working on a QA scrip for the PHP documentation,
was not understanding why CI was failing with no output,
until you realised that the issue was me passing a boolean to exit() that was being cast to a string and not the expected integer.
So I would argue you have stumbled upon this.
Perhaps I've missed some pros, but it seems these have been the ones the
RFC explicitly mentions.So, what are the cons:
* removing the ZEND_EXIT opcode
That immediately breaks any extension using it. A quick Github search
lists 3.1 k occurrences[1].
Please see point about the stream layer API change.
* making exit a "proper" function
It still can be called without parentheses, so I don't regard it as a
proper function. It might even be confusing to readers ("oh, now I
can call a function without arguments by omitting the parentheses – but
why doesn't it work for other functions?"). And the PHP manual even
states (emphasis mine):| exit is a language construct and it can be called without
| parentheses if no status is passed.Weighting the pros and cons is left as an exercise to the reader.
Since when has the documentation been the normative aspect of PHP?
We routinely fix the documentation to explain the Zend engine's true behaviour, even if it doesn't make any sense.
This sentence is true as of now, and may be changed if this RFC passes.
Using current wording on the documentation is.... a strange argument to me.
Best regards,
Gina P. Banyard
[1] php-src/UPGRADING.INTERNALS at PHP-8.1 · php/php-src · GitHub
On 06.08.2024 at 21:07, Gina P. Banyard wrote:
On Monday, 5 August 2024 at 23:55, Christoph M. Becker <cmbecker69@gmx.de> wrote:
Most likely, yes, although still somebody has to provide a fix, and
someone has to do a new release.Why should I investigate time to fix extensions when this RFC might not even land in the first place?
I do not expect anybody who makes approved changes to the APIs to
provide fixes for any of the affected external extensions. It's great
if they help when asked, and it's okay if they don't.
I merely noted that someone has to make these changes to keep these
extensions up-to-date for new PHP releases. And sometimes real life
interferes for the maintainers, so they don't have time (due to other
interests, health issues, a new job, family, etc.) And even if others
would like to help out, sometimes they can't[1].
We do countless engine breaking changes without RFCs and without notices to extension maintainers other than UPGRADING.INTERNALS.
That is exactly my point: we should weight the pros and cons of any API
change carefully. How many extensions may be affected, how hard would
it be to update these extensions, how ugly would the resulting code be
due to PHP_VERSION_ID checks, etc. And on the other hand, what would be
the benefits for the whole ecosystem (php-src, extensions, userland
developers).
Could we maybe start respecting the intelligence of extension developers?
And if not, can we stop pretending that we care about them when we, as a project, clearly don't seem to care in our actual action?
We should not fall into the trap of thinking: we did more seriously
breaking changes in the past, so we don't need to care anymore. After
all, we cannot change what had happened, but we may be able to improve.
So we can now write `exit(status = 1)` or `exit(status = "some error message`). I don't see any improvement using named arguments for a
single argument function (maybe unless it is a boolean argument). And
for a string argument, I don't think that `status` is a sensible
parameter name here.Please propose a better name then.
That's the crux with some of the union types for legacy functions: there
are no suitable names. setcookie() has an $expires_or_options
parameter, so we might choose $status_or_message here, and although that
seems to be more correct, it would be ridiculous to use this as named
argument.
(For the record: I've got the syntax wrong, because I don't use named
arguments, since these are fixing the symptoms, not the desease, which
is a bad API.)
That might be a nice feature, but at least I have never seen the need to
pass `exit` as callable. And it wouldn't be hard to define a wrapper
function (e.g. `my_exit()`) if someone really needs to pass it as a
callable.The fact that one might need to do this workaround is bad already, and not a justification against this RFC.
The fact that one might need to pass `exit` implies that they are using
some test helper extension (e.g. uopz) to stub/mock it, or that they
should consider to consult a therapist. 
That might be an issue (although I've never stumbled upon that), but
it seems to me that this could even be handled in the ZEND_EXIT handler.Now see this is strange to me, as I clearly remember hitting this issue while working on a QA scrip for the PHP documentation,
was not understanding why CI was failing with no output,
until you realised that the issue was me passing a boolean to exit() that was being cast to a string and not the expected integer.
I have completely forgotten about this, and cannot really remember even now.
So I would argue you have stumbled upon this.
I stand corrected. However, that doesn't invalidate my argument that at
least some sanity could be introduced in the ZEND_EXIT handler instead
of pursuing the RFC at hand.
It still can be called without parentheses, so I don't regard it as a
proper function. It might even be confusing to readers ("oh, now I
can call a function without arguments by omitting the parentheses – but
why doesn't it work for other functions?"). And the PHP manual even
states (emphasis mine):| exit is a language construct and it can be called without
| parentheses if no status is passed.Since when has the documentation been the normative aspect of PHP?
We routinely fix the documentation to explain the Zend engine's true behaviour, even if it doesn't make any sense.This sentence is true as of now, and may be changed if this RFC passes.
Using current wording on the documentation is.... a strange argument to me.
Fair enough. So just ignore that last part, but what about:
It still can be called without parentheses, so I don't regard it as a
proper function. It might even be confusing to readers ("oh, now I
can call a function without arguments by omitting the parentheses – but
why doesn't it work for other functions?").
Anyhow, perhaps an alternative to the proposal at hand would be to
implement the following proper function in the core:
function quit(int $status = 0) {
exit($status);
}
Users who wish to have something like die("message"), could use it, or
echo "message";
quit()
Or perhaps that could be a set of quit functions; the main point is not
to *change* anything in the core (for now), but still allowing users to
*opt into* some cleaner functionality.
[1] <Update dead link to Fedora package by AndrewFeeney · Pull Request #103 · krakjoe/pcov · GitHub;
Cheers,
Christoph