[PHP-DEV] [RFC] [VOTE] Transform exit() from a language construct into a standard function

Mailing Lists php.internals
21

On Tue, July 30, 2024 at 03:49 G. P. Banyard 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-function

The vote will last for two weeks until the 13th of August 2024.

I really appreciate RFCs like this which not only make the language more consistent for userland developers, but also simplify PHP's internal implementation, paving the way for future optimizations and new functionality.

In my experience, extension developers nearly always have to make some changes to support each new PHP version, so I'm not sure why that would be a reason to prevent improving the language.

Best regards,
Theodore

22

On 7 August 2024 14:30:24 BST, Theodore Brown <theodorejb@outlook.com> wrote:

On Tue, July 30, 2024 at 03:49 G. P. Banyard 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-function

The vote will last for two weeks until the 13th of August 2024.

I really appreciate RFCs like this which not only make the language more consistent for userland developers, but also simplify PHP's internal implementation, paving the way for future optimizations and new functionality.

In my experience, extension developers nearly always have to make some changes to support each new PHP version, so I'm not sure why that would be a reason to prevent improving the language.

This is misrepresenting my concern.

I understand that new versions require changes.

One of my issues is, is that so far I could not find a way to replicate existing functionality with this patch applied.

The RFC does not mention a BC break, nor does it have an entry for UPGRADING.INTERNALS either.

cheers
Derick

23

Stupid question maybe, but are we voting on the RFC or on the patch?

If the patch does not match what.the RFC proposes, then the patch has a problem. That should IMO though not affect voting on an RFC.

Or am I.missimg something?

Cheers

Andreas

On 7 August 2024 16:27:56 CEST, Derick Rethans derick@php.net wrote:


On 7 August 2024 14:30:24 BST, Theodore Brown <theodorejb@outlook.com> wrote:

> On Tue, July 30, 2024 at 03:49 G. P. Banyard wrote:
> 
> > 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](https://wiki.php.net/rfc/exit-as-function)
> > 
> >  The vote will last for two weeks until the 13th of August 2024.
> 
> I really appreciate RFCs like this which not only make the language more consistent for userland developers, but also simplify PHP's internal implementation, paving the way for future optimizations and new functionality.
> 
> In my experience, extension developers nearly always have to make some changes to support each new PHP version, so I'm not sure why that would be a reason to prevent improving the language.

This is misrepresenting my concern.

I understand that new versions require changes. 

One of my issues is, is that so far I could not find a way to replicate existing functionality with this patch applied.

The RFC does not mention a BC break, nor does it have an entry for UPGRADING.INTERNALS either.

cheers
Derick
24

On Wed, 7 Aug 2024, 16:09 Andreas Heigl, <andreas@heigl.org> wrote:

Stupid question maybe, but are we voting on the RFC or on the patch?

I have been reliably informed that votes are always for RFCs.

Cheers,
Bilge

25

On Wed, August 7, 2024 at 08:27 Derick Rethans wrote:

On 7 August 2024 14:30:24 BST, Theodore Brown wrote:

I really appreciate RFCs like this which not only make the language more consistent for userland developers, but also simplify PHP's internal implementation, paving the way for future optimizations and new functionality.

In my experience, extension developers nearly always have to make some changes to support each new PHP version, so I'm not sure why that would be a reason to prevent improving the language.

This is misrepresenting my concern.

I understand that new versions require changes.

One of my issues is, is that so far I could not find a way to replicate existing functionality with this patch applied.

The RFC does not mention a BC break, nor does it have an entry for UPGRADING.INTERNALS either.

Hi Derick,

I'm confused by this, since earlier in the thread Tim responded with examples showing how the behavior of exit() can still be observed and correctly handled. If that didn't address your issue, can you explain it further? It seems like right now everyone is left bewildered about what functionality you couldn't replicate.

Kind regards,
Theodore

26

On Wednesday, 7 August 2024 at 17:07, Andreas Heigl <andreas@heigl.org> wrote:

Stupid question maybe, but are we voting on the RFC or on the patch?

If the patch does not match what.the RFC proposes, then the patch has a problem. That should IMO though not affect voting on an RFC.
Or am I.missimg something?

In theory, it is the RFC idea.
In practice, a lot of the times it is the patch for complex features.

However, it is still within the purview of core developers to veto the implementation of an RFC.
Which could be the case here rather than voting against the RFC outright.

Best regards,
Gina P. Banyard

27

Hey Gina, hey all

Am 08.08.24 um 15:44 schrieb Gina P. Banyard:

On Wednesday, 7 August 2024 at 17:07, Andreas Heigl <andreas@heigl.org> wrote:

Stupid question maybe, but are we voting on the RFC or on the patch?

If the patch does not match what.the RFC proposes, then the patch has a problem. That should IMO though not affect voting on an RFC.

Or am I.missimg something?

In theory, it is the RFC idea.
In practice, a lot of the times it is the patch for complex features.

However, it is still within the purview of core developers to veto the implementation of an RFC.
Which could be the case here rather than voting against the RFC outright.

I have no problem that core developers veto a certain implementation of an RFC. I actually expect them to do so.

But the vote should IMO *always and exclusively* be based on the RFC. Not on the implementation. If the voting happens based on the implementation due to the complexity of the features that means that the RFC is not wel written and needs to be improved. Or the implementation is problematic and needs to be vetoed by the core developers.

Why do I think so? It becomes completely intransparent why an RFC was rejected when the voting happens based on a meager implementation as after some years no one will understand why a well written RFC was rejected. Especially when the discussion then also happens off-list and on the actual code in github as that tears apart the information sources that need to be taken into consideration in hindsight.

Also the RFC should specify the implementation. If the implementation doesn't match the RFC then the implementation is faulty and needs to change until it *does* match the RFC. But that is an implementation detail. The voting though should only happen on the description of the feature.

My 0.02 €

Cheers

Andreas
--
                                                               ,
                                                              (o o)
+---------------------------------------------------------ooO-(_)-Ooo-+
| Andreas Heigl |
| mailto:andreas@heigl.org N 50°22'59.5" E 08°23'58" |
| https://andreas.heigl.org |
+---------------------------------------------------------------------+
| Appointments - Nextcloud |
+---------------------------------------------------------------------+
| GPG-Key: https://hei.gl/keyandreasheiglorg |
+---------------------------------------------------------------------+

28

Hi

On 8/7/24 22:33, Theodore Brown wrote:

I'm confused by this, since earlier in the thread Tim responded with examples showing how the behavior of exit() can still be observed and correctly handled. If that didn't address your issue, can you explain it further? It seems like right now everyone is left bewildered about what functionality you couldn't replicate.

Tideways generously sponsored 2 hours of my time to develop a proof of concept patch to fix Xdebug against the current version of Gina's PR. Much of that time was spent chasing a reference counting issue in an unfamiliar codebase. Unfortunately ASAN was not of much help, because running Xdebug's tests against PHP 8.4 reported some pre-existing memory leaks.

I have attached two patches for Xdebug:

0001-Support-exit-as-function.patch:

This patch fixes the build of Xdebug, by removing the references to ZEND_EXIT for PHP 8.4.

It also adds a hook on the new exit() function to "deinitialize" Xdebug, which will ensure that profiling files are written for the use in CI. My understanding is that an alternative solution would have been to add a special `xdebug_finalize_profile()` function for use in CI instead of overloading the exit() function.

All the existing tests in Xdebug's codebase pass after this patch, except that I did not adjust the profiler test expectations to take the the new exit() function call that was not previously there into account, because I do not fully understand the output format of the tests. Nevertheless the changes to the test output looked correct to me.

0002-Exit-coverage.patch:

This patch adds support for the coverage analysis, so that Xdebug understands that any code after a call to exit() is unreachable. As far as I can tell this case was not previously tested by any of Xdebug's tests. I have thus added a test case, but I don't fully understand the output format of the coverage tests, thus I may have overlooked something. What I did verify is that the test fails when I remove the implementation from code_coverage.c.

I'd like to note that this patch is not strictly necessary for Xdebug to be usable by a user. It might erroneously report unreachable code as reachable, but by definition unreachable code is useless. Thus a user of Xdebug could work around this by simply removing the unreachable code. My understanding is that some userland static analyzers and refactoring tools are already capable of pointing out such unreachable code.

--------

Given my unfamiliarity of Xdebug's codebase and the limited time I spent preparing these patches, they should be considered a proof of concept only. They might or might not match the preferred coding style used in the Xdebug codebase. It might also be possible that they did not correctly handle an edge case, due to my lack of knowledge about the test infrastructure.

Nevertheless I believe that someone more familiar with the codebase will certainly be able to adapt these patches as necessary to make Xdebug fully compatible with Gina's PR.

In any case I believe they clearly showcase that it is not impossible to keep the existing functionality, even with Gina's RFC.

Best regards
Tim Düsterhus

(Attachment 0001-Support-exit-as-function.patch is missing)

(Attachment 0002-Exit-coverage.patch is missing)

29

Hi

On 8/6/24 00:55, Christoph M. Becker wrote:

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].

For me this is a pro, because it makes the implementation simpler going forward: The engine, the JIT and extensions have one fewer Opcode they need to deal with / take into account / test. Anything that is capable of handling functions that throw exceptions will be capable of handling the proposed exit() function.

Best regards
Tim Düsterhus

30

On Thu, Aug 8, 2024, at 16:10, Andreas Heigl wrote:

Hey Gina, hey all

Am 08.08.24 um 15:44 schrieb Gina P. Banyard:

On Wednesday, 7 August 2024 at 17:07, Andreas Heigl <andreas@heigl.org>

wrote:

Stupid question maybe, but are we voting on the RFC or on the patch?

If the patch does not match what.the RFC proposes, then the patch has

a problem. That should IMO though not affect voting on an RFC.

Or am I.missimg something?

In theory, it is the RFC idea.

In practice, a lot of the times it is the patch for complex features.

However, it is still within the purview of core developers to veto the

implementation of an RFC.

Which could be the case here rather than voting against the RFC outright.

I have no problem that core developers veto a certain implementation of

an RFC. I actually expect them to do so.

But the vote should IMO always and exclusively be based on the RFC.

Not on the implementation. If the voting happens based on the

implementation due to the complexity of the features that means that the

RFC is not wel written and needs to be improved. Or the implementation

is problematic and needs to be vetoed by the core developers.

Why do I think so? It becomes completely intransparent why an RFC was

rejected when the voting happens based on a meager implementation as

after some years no one will understand why a well written RFC was

rejected. Especially when the discussion then also happens off-list and

on the actual code in github as that tears apart the information sources

that need to be taken into consideration in hindsight.

I would expect any implementation done before the RFC is voted on to be entirely proof-of-concept, and not expected to be mergable as-is. Basically, a way to test out the new proposed RFC, but may have issues (such as memory leaks or not all edge cases implemented). I, personally, wouldn’t expect an RFC to be declined based on the initial patch. That’s good information to add to the rfc how-to page.

— Rob

31

On Thu, Aug 8, 2024, at 9:10 AM, Andreas Heigl wrote:

Hey Gina, hey all

Am 08.08.24 um 15:44 schrieb Gina P. Banyard:

On Wednesday, 7 August 2024 at 17:07, Andreas Heigl <andreas@heigl.org>
wrote:

Stupid question maybe, but are we voting on the RFC or on the patch?

If the patch does not match what.the RFC proposes, then the patch has
a problem. That should IMO though not affect voting on an RFC.

Or am I.missimg something?

In theory, it is the RFC idea.
In practice, a lot of the times it is the patch for complex features.

However, it is still within the purview of core developers to veto the
implementation of an RFC.
Which could be the case here rather than voting against the RFC outright.

I have no problem that core developers veto a certain implementation of
an RFC. I actually expect them to do so.

But the vote should IMO *always and exclusively* be based on the RFC.
Not on the implementation. If the voting happens based on the
implementation due to the complexity of the features that means that the
RFC is not wel written and needs to be improved. Or the implementation
is problematic and needs to be vetoed by the core developers.

How exactly would voters veto an implementation if not through the RFC? That's literally the only formal input mechanism they have, and previous attempts to add others have been soundly rejected.

As a historical note, the partial function application RFC was declined despite there being general consensus that the proposal was quite good and quite desireable. The issue was that Nikita felt the implementation proposed with it was too fragile, and wasn't sure how to make it less fragile, so he voted No and several others followed suit. I am fairly confident that if a less-fragile implementation could be found, it would pass handily.

So yes, RFCs have been rejected in the past on "implementation only."

--Larry Garfield

32

On Friday, 9 August 2024 at 16:03, Larry Garfield <larry@garfieldtech.com> wrote:

How exactly would voters veto an implementation if not through the RFC? That's literally the only formal input mechanism they have, and previous attempts to add others have been soundly rejected.

As a historical note, the partial function application RFC was declined despite there being general consensus that the proposal was quite good and quite desireable. The issue was that Nikita felt the implementation proposed with it was too fragile, and wasn't sure how to make it less fragile, so he voted No and several others followed suit. I am fairly confident that if a less-fragile implementation could be found, it would pass handily.

So yes, RFCs have been rejected in the past on "implementation only."

The implementation for the attribute syntax using @ (before it got revoted) would have been vetoed due to implementation issues if the "Treat namespaced names as single token" RFC wouldn't have been accepted.

I am very much aware that RFCs have been rejected on implementation, but even if an RFC is accepted it can be vetoed by core developers.

And once again, I still do not have any idea why Derick has issues adding support to XDebug.
Especially as Tim seemingly managed to do this while being unfamiliar with the codebase.

Best regards,

Gina P. Banyard

33

On Tuesday, 30 July 2024 at 11:49, 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-function

The vote will last for two weeks until the 13th of August 2024.

Best regards,

Gina P. Banyard

Hello Internals,

I just closed the vote.

The final results are 24 votes in favour and 12 against.
Meaning it has reached the 2/3 threshold required for an RFC to be accepted.
This is closer than I'd like, but I guess I'll take it.

Thank you to everyone that has voted.

Best regards,

Gina P. Banyard

34

On 13 Aug 2024, at 12:36, Gina P. Banyard <internals@gpb.moe> wrote:

On Tuesday, 30 July 2024 at 11:49, 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-function

The vote will last for two weeks until the 13th of August 2024.

Best regards,

Gina P. Banyard

Hello Internals,

I just closed the vote.

The final results are 24 votes in favour and 12 against.
Meaning it has reached the 2/3 threshold required for an RFC to be accepted.
This is closer than I'd like, but I guess I'll take it.

Thank you to everyone that has voted.

Best regards,

Gina P. Banyard

Hi,

Were there more discussions on this off the list? I presume there are often discussions elsewhere between the core group of internal developers.

It's just I never saw a confirmed answer on the Xdebug issue but did see a significant drop off in no votes without any further explanation (appreciating that no-one has to give an explanation) in the last couple of days.

Did the issue get resolved?

Thanks very much,

Matt

35

On Tuesday, 13 August 2024 at 14:05, Matthew Sewell <php@sewell.info> wrote:

> On 13 Aug 2024, at 12:36, Gina P. Banyard internals@gpb.moe wrote:
>
> On Tuesday, 30 July 2024 at 11:49, 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-function
> >
> > The vote will last for two weeks until the 13th of August 2024.
> >
> > Best regards,
> >
> > Gina P. Banyard
>
> Hello Internals,
>
> I just closed the vote.
>
> The final results are 24 votes in favour and 12 against.
> Meaning it has reached the 2/3 threshold required for an RFC to be accepted.
> This is closer than I'd like, but I guess I'll take it.
>
> Thank you to everyone that has voted.
>
> Best regards,
>
> Gina P. Banyard

Hi,

Were there more discussions on this off the list? I presume there are often discussions elsewhere between the core group of internal developers.

It's just I never saw a confirmed answer on the Xdebug issue but did see a significant drop off in no votes without any further explanation (appreciating that no-one has to give an explanation) in the last couple of days.

Did the issue get resolved?

Thanks very much,

Matt

Derick has told me off list that Tim's patches look reasonable.

Best regards,

Gina P. Banyard

36

Hi

On 8/13/24 14:43, Gina P. Banyard wrote:

Derick has told me off list that Tim's patches look reasonable.

There's also this PR for Xdebug that includes my two patches: Exit as function by derickr · Pull Request #972 · xdebug/xdebug · GitHub (and some additional fixes on top of them, because, as I said, I did not clean them up).

Best regards
Tim Düsterhus

37

On 13 Aug 2024, at 13:43, Gina P. Banyard <internals@gpb.moe> wrote:

On Tuesday, 13 August 2024 at 14:05, Matthew Sewell <php@sewell.info> wrote:

On 13 Aug 2024, at 12:36, Gina P. Banyard internals@gpb.moe wrote:

On Tuesday, 30 July 2024 at 11:49, 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-function

The vote will last for two weeks until the 13th of August 2024.

Best regards,

Gina P. Banyard

Hello Internals,

I just closed the vote.

The final results are 24 votes in favour and 12 against.
Meaning it has reached the 2/3 threshold required for an RFC to be accepted.
This is closer than I'd like, but I guess I'll take it.

Thank you to everyone that has voted.

Best regards,

Gina P. Banyard

Hi,

Were there more discussions on this off the list? I presume there are often discussions elsewhere between the core group of internal developers.

It's just I never saw a confirmed answer on the Xdebug issue but did see a significant drop off in no votes without any further explanation (appreciating that no-one has to give an explanation) in the last couple of days.

Did the issue get resolved?

Thanks very much,

Matt

Derick has told me off list that Tim's patches look reasonable.

Best regards,

Gina P. Banyard

Thank you very much for the response (and the RFC!).

Best wishes,

Matt