Re: RFC: Expectations

Morning, I hear the concern regarding custom exceptions. Things will be used badly whatever. It's easy to imagine that in a simple application you just don't need to specify custom exceptions. But in a large codebase, being able to structure exceptions is invaluable, it gives documentation reference points and makes stack traces easier to read at a glance, because they are more meaningful. So I think we should keep the ability to throw custom exceptions. I agree about making the exception part of a new tree, so that they are not caught, so I'll just wait for that from dmitry and open the vote. Cheers Joe On Wed, Feb 18, 2015 at 5:01 PM, Dmitry Stogov <dmitry@zend.com> wrote: > > > On Wed, Feb 18, 2015 at 5:44 PM, Nikita Popov <nikita.ppv@gmail.com> > wrote: > >> On Tue, Feb 17, 2015 at 10:50 PM, Dmitry Stogov <dmitry@zend.com> wrote: >> >>> Hi Joe >>> >>> The patch is ready https://github.com/php/php-src/pull/1088/files�� >>> �]��~M��Z�� >>> >>> 1) I implemented AST pretty-printer to reconstruct the source. It may be >>> reused in Reflection and other places through >>> >>> ZEND_API zend_string *zend_ast_export(const char *prefix, zend_ast *ast, >>> const char *suffix); >>> >>> 2) zend.assertions=-1 - makes zero-cost asserts >>> >>> 3) assert() in a namespace leads to call a function defined in this >>> namespace (if it's defined), but zend.assertions is still may disable this >>> call or even prevent code generation for it. it's possible to use \assert() >>> to call the system function. >>> >>> Please, make update RFC, add notes about (2) and (3). >>> Then, it should be ready for voting. >>> >>> Nikita, please take a quick look over the patch. I hope, you don't have >>> objections. >>> >> >> I've added a few comments on the PR. >> > > Thank you very much. You found about 25 bugs :) > All of them except for "elseif" should be fixed now. > I also think printing "else if" instead of "elseif" is not a big problem. > Pretty-printer may also add or remove brackets in some situations. > > >> >> Two general notes on the RFC: >> >> 1. I don't like the ability to specify a different exception as the >> second param. Assertions are supposed to be used as sanity checks during >> development, not to throw meaningful and specialized exception types. >> Having this possibility will probably only encourage bad usage of assert(). >> It's not a big problem to me, but I'd rather not have this "feature". >> > > Joe, this is part of your old patch. I really don't care about it. > > >> 2. Similar to the EngineExceptions RFC I'm wondering if >> AssertionException should extend Exception or be in a separate hierarchy. >> The same argument as with engine exceptions applies: It's pretty unlikely >> that you want to catch an AssertionException anywhere apart from top-level >> error handling code and that people using catch(Exception) blocks would >> accidentally catch assertions. I'm not sure I agree with this, but I wanted >> to mentioned the concern. >> > > This may be changed together with EngineException patch. > I started working on it, and I hope I'll show you results tomorrow. > > Thanks. Dmitry. > > >> >> Nikita >> >> >