On Thu, Feb 19, 2015 at 4:13 PM, Dmitry Stogov <dmi...@zend.com> wrote:
> Hi Nikita, > > I refactored your implementation: https://github.com/php/php-src/pull/1095 > > I introduced a class hierarchy to minimize effect on existing code. > cacth (Exception $e) won't catch new types of exceptions. > > BaseException (abstarct) > +- EngineException > +- ParaseException > +- Exception > +-ErrorException > +- all other exceptions > > In case of uncaught Parse and EngineEexception PHP writes the compatible > error message. > I made it mainly to keep thousand PHPT tests unchanged. > If you like we may introduce an ini option that will lead to emitting of > "Uncaught Exception ... " with backtrace instead. > > The internal API was changed a bit. e.g. EngineException are thrown from > zend_error(), if the error_code has E_EXCEPTION bit set. > So to change some error into exception now we should change > zend_error(E_ERROR,...) into zend_error(E_EXCEPTION|E_ERROR. ...) > > All tests are passed. > I'm not sure about sapi/cli/tests/bug43177.phpt, because it started to > fail also in master. > > We may need to replace E_RECOVERABLE_ERROR with E_ERROR and fix > corresponding tests. > Despite of this, I think the patch is good enough to be merged into master. > > We may decide to convert more fatal errors later, but it shouldn't prevent > from putting RFC into vote. > > Thoughts? > I've updated some minor points in the RFC to be consistent with the new patch. The BaseException based hierarchy will be a separate vote [1]. If there are no further concerns I'd start voting on this RFC tomorrow. One point wrt the patch: Do you think it would be hard to use a clean shutdown on uncaught exceptions? This would make sure we don't forget to free anything when throwing exceptions. Nikita [1]: https://wiki.php.net/rfc/engine_exceptions_for_php7#hierarchy