Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:123450
Date: Tue, 28 May 2024 17:47:58 +0100 (BST)
To: "Gina P. Banyard" <internals@gpb.moe>
cc: Ilija Tovilo <tovilo.ilija@gmail.com>, 
    PHP internals <internals@lists.php.net>
Subject: Re: [PHP-DEV] [RFC] Transform exit() from a language construct into
 a standard function
In-Reply-To: <vdu-94cPOz_778NDf4shhlPHZqiBu1EqnPHqjqtBizxYfMJT_K2phGTjCIFUjbfeV3425xUoFUazlkbjdUt4A9t2tZ_XBgx9Y7MiLiwoOdE=@gpb.moe>
Message-ID: <6c6c2a6a-fc8d-71c6-9efa-a8eab2d8f2fd@php.net>
References: <GUMp_LJKefk4aLz9P9MA-zonKuik6uUNWZTtOAx-IeIhjPFsgfT3HHECPpyGF_ujq-LxI6tzBxMYgL5CMVGRkyzxSugweKix_YUGo2rXnb4=@gpb.moe> <b2IQjZQlOsMoMuk9Jw5edeVfHqqbcWCQc6DQF5jKrRkGryVyvu6rC9sbSW2D-HK9jH2QUmkoIy--aQROFsx-3QPr8CjIfltTnhbZYGxsixI=@gpb.moe>
 <CAPyj-LBpmYv30JFW3x-ZL=28vivrh7d04L97rbXeYxDJirwKCg@mail.gmail.com> <ca1189e2-4bb7-3008-4b52-694cd14aed8d@php.net> <vdu-94cPOz_778NDf4shhlPHZqiBu1EqnPHqjqtBizxYfMJT_K2phGTjCIFUjbfeV3425xUoFUazlkbjdUt4A9t2tZ_XBgx9Y7MiLiwoOdE=@gpb.moe>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323329-1482575356-1716914878=:61845"
From: derick@php.net (Derick Rethans)

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323329-1482575356-1716914878=:61845
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: QUOTED-PRINTABLE

On Tue, 28 May 2024, Gina P. Banyard wrote:

> On Tuesday, 28 May 2024 at 15:26, Derick Rethans <derick@php.net> wrote:
>=20
> > On Mon, 27 May 2024, Ilija Tovilo wrote:
> >=20
> > > On Sun, May 26, 2024 at 11:47=E2=80=AFPM Gina P. Banyard internals@gp=
b.moe=20
> > > wrote:
> > >=20
> > > > On Wednesday, 8 May 2024 at 14:40, Gina P. Banyard=20
> > > > internals@gpb.moe wrote:
> > > >=20
> > > > > I would like to formally propose my idea for exit() as a=20
> > > > > function brought up to the list on 2024-02-24 [1] with the=20
> > > > > following RFC: https://wiki.php.net/rfc/exit-as-function
> > > >=20
> > > > As there haven't been any comments for nearly two weeks, I'm=20
> > > > planning on opening the vote for the RFC on Tuesday.
> >=20
> >=20
> > Right now, I am not going to be in favour of this. Specfically=20
> > because the Backward Incompatible Changes don't list some of the=20
> > backward breaking changes, and no mitigations are provided. See=20
> > below.
> >=20
> > I also don't see the benefit from doing this in the first place.=20
> > exit/die has always meant "bail straight out", without any function=20
> > semantics (from https://www.php.net/exit):
> >=20
> > exit =E2=80=94 Output a message and terminate the current script
> >=20
> > exit is a language construct and it can be called without=20
> > parentheses if no status is passed.
> >=20
> > It's not that this a particularly new or novel behaviour.
> >=20
> > I saw somewhere else in the thread that the exit construct could=20
> > already throw a catchable exception. I consider that a bug.
>=20
> Considering the whole point of the RFC is to make it a function, I'm=20
> not sure why saying the current exit behaviour is not new or novel=20
> serves any purpose.

I understand that that is the point, but I don't think it's argued *why*=20
that needs to happen.

> Is catching a TypeError a bug? Is catching a ValueError a bug? Is=20
> catching any sort of engine Error a bug?

exit throwing any Exception is the bug I was referring to.

> Because this is effectively what you are saying, and if this is the=20
> case, we might as well just reverse the "Exceptions in the engine" RFC=20
> from 2014 [1] to make all those Errors fatal errors again which bail=20
> out immediately. This RFC makes exit() a function such that either it=20
> exits normally, or a TypeError is thrown if you feed it nonsense,=20
> consistently with all our type juggling semantics.
>=20
> Moreover, when exit() was changed to use an Exception internally there=20
> were various people in favour of being able to catch it, and have it=20
> run finally blocks (like other programming languages do) [2] It should=20
> also be noted exit() does not just bail out and do nothing else, any=20
> destructors will be run before the exiting, so one can still run=20
> userland code after it has been called [3]
>=20
> > I have just checked this for Xdebug, and you're definitely right=20
> > with that. With the removal of the ZEND_EXIT opcode, it can not now=20
> > detect a scope/function exit now.
> >=20
> > It also breaks my "do tasks on ZEND_EXIT" with the profiler. It is=20
> > used to always write the closing profiling footer to the files.=20
> > Without me being able to overload thati opcode, data would not be=20
> > complete. I even have two bugs (with tests) for this:
> >=20
> > - https://bugs.xdebug.org/68
> > - https://bugs.xdebug.org/631
> >=20
> > Xdebug has been overloading ZEND_EXIT for nearly 20 years now.
>=20
> AFAIK Opcodes are not part of any backwards compatibility guarantees.

AFAIK it isn't documented otherwise either. Just because it's not=20
documented, doesn't mean we should remove long term existing=20
functionality.

> You could overwrite the function pointer of exit() at MINIT with a=20
> custom implementation.

I can, but overriding functions for this sort of thing is a little bit=20
of a hack, and I prefer having an actual API. It is also a runtime=20
feature, not a compile time (see below again).

> Alternatively, you could use the zend_is_unwind_exit() engine API to=20
> check if the exception that has been thrown is the one that=20
> corresponds to an exit call, this would work as of PHP 8.0.

> Finally, the one that makes more work for me, is to add a new observer=20
> API that indicates an exit() and hook into that one.

That seems a fairly sensible thing to add. But that only addresses my=20
profiler use case, and not the branch/path detection use case.

> >=20
> > > This could be re-added by checking for the never return type instead.
> >=20
> >=20
> > I can't quite see a way on how to do that? The EXIT is replaced by an
> > INIT_FCALL/DO_ICALL:
> >=20
> >      4     0  E >   EXT_STMT
> >            1        ECHO           '55'
> >      5     2        EXT_STMT
> > -          3      > EXIT
> > -    6     4*       EXT_STMT
> > -          5*       ECHO           '675'
> > -    7     6*       EXT_STMT
> > -          7*     > RETURN         null
> > +          3        INIT_FCALL     'exit'
> > +          4        DO_ICALL
> > +    6     5        EXT_STMT
> > +          6        ECHO           '675'
> > +    7     7        EXT_STMT
> > +          8      > RETURN         null
> >=20
> >=20
> > The opcodes don't have direct access to the type mask structure as=20
> > far as I know.
>=20
> You can access the zend_function pointer from the opcode which has=20
> access to the type-mask info.

Isn't that only during runtime, and not compile time?

The branch/path detection works with the op_array, just after it has=20
been created through compile_file. I don't see any structure in the=20
DO_ICALL opcode that has access to this never flag.

> Or, if required, the Zend/Optimizer/zend_func_infos.h file can be=20
> amended to include exit() and die() and expose the MAY_BE_NEVER return=20
> type.

I don't think that helps either, as that is also run time?

cheers,
Derick
--8323329-1482575356-1716914878=:61845--