On Tue, 28 May 2024, Gina P. Banyard wrote: > On Tuesday, 28 May 2024 at 15:26, Derick Rethans <der...@php.net> wrote: > > > On Mon, 27 May 2024, Ilija Tovilo wrote: > > > > > On Sun, May 26, 2024 at 11:47 PM Gina P. Banyard intern...@gpb.moe > > > wrote: > > > > > > > On Wednesday, 8 May 2024 at 14:40, Gina P. Banyard > > > > intern...@gpb.moe wrote: > > > > > > > > > I would like to formally propose my idea for exit() as a > > > > > function brought up to the list on 2024-02-24 [1] with the > > > > > following RFC: https://wiki.php.net/rfc/exit-as-function > > > > > > > > As there haven't been any comments for nearly two weeks, I'm > > > > planning on opening the vote for the RFC on Tuesday. > > > > > > Right now, I am not going to be in favour of this. Specfically > > because the Backward Incompatible Changes don't list some of the > > backward breaking changes, and no mitigations are provided. See > > below. > > > > I also don't see the benefit from doing this in the first place. > > exit/die has always meant "bail straight out", without any function > > semantics (from https://www.php.net/exit): > > > > exit — Output a message and terminate the current script > > > > exit is a language construct and it can be called without > > parentheses if no status is passed. > > > > It's not that this a particularly new or novel behaviour. > > > > I saw somewhere else in the thread that the exit construct could > > already throw a catchable exception. I consider that a bug. > > Considering the whole point of the RFC is to make it a function, I'm > not sure why saying the current exit behaviour is not new or novel > serves any purpose.
I understand that that is the point, but I don't think it's argued *why* that needs to happen. > Is catching a TypeError a bug? Is catching a ValueError a bug? Is > 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 > case, we might as well just reverse the "Exceptions in the engine" RFC > from 2014 [1] to make all those Errors fatal errors again which bail > out immediately. This RFC makes exit() a function such that either it > exits normally, or a TypeError is thrown if you feed it nonsense, > consistently with all our type juggling semantics. > > Moreover, when exit() was changed to use an Exception internally there > were various people in favour of being able to catch it, and have it > run finally blocks (like other programming languages do) [2] It should > also be noted exit() does not just bail out and do nothing else, any > destructors will be run before the exiting, so one can still run > userland code after it has been called [3] > > > I have just checked this for Xdebug, and you're definitely right > > with that. With the removal of the ZEND_EXIT opcode, it can not now > > detect a scope/function exit now. > > > > It also breaks my "do tasks on ZEND_EXIT" with the profiler. It is > > used to always write the closing profiling footer to the files. > > Without me being able to overload thati opcode, data would not be > > complete. I even have two bugs (with tests) for this: > > > > - https://bugs.xdebug.org/68 > > - https://bugs.xdebug.org/631 > > > > Xdebug has been overloading ZEND_EXIT for nearly 20 years now. > > AFAIK Opcodes are not part of any backwards compatibility guarantees. AFAIK it isn't documented otherwise either. Just because it's not documented, doesn't mean we should remove long term existing functionality. > You could overwrite the function pointer of exit() at MINIT with a > custom implementation. I can, but overriding functions for this sort of thing is a little bit of a hack, and I prefer having an actual API. It is also a runtime feature, not a compile time (see below again). > Alternatively, you could use the zend_is_unwind_exit() engine API to > check if the exception that has been thrown is the one that > 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 > API that indicates an exit() and hook into that one. That seems a fairly sensible thing to add. But that only addresses my profiler use case, and not the branch/path detection use case. > > > > > This could be re-added by checking for the never return type instead. > > > > > > I can't quite see a way on how to do that? The EXIT is replaced by an > > INIT_FCALL/DO_ICALL: > > > > 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 > > > > > > The opcodes don't have direct access to the type mask structure as > > far as I know. > > You can access the zend_function pointer from the opcode which has > 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 been created through compile_file. I don't see any structure in the DO_ICALL opcode that has access to this never flag. > Or, if required, the Zend/Optimizer/zend_func_infos.h file can be > amended to include exit() and die() and expose the MAY_BE_NEVER return > type. I don't think that helps either, as that is also run time? cheers, Derick
