Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:83071
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.46 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CA+9eiLs67UBr7DZ2QmUWSijih7bOPhHbqSu=gvVAPphQ5G-APQ@mail.gmail.com>
References: <CAL=_i_=cYXf=yjy7y6pytg7M=pe5z8dKpriqJiG2mC_XiMXYXw@mail.gmail.com>
	<54E1C24B.6010806@googlemail.com>
	<CAL=_i_kuuj-q6L0n4vZC2BC_D5Y_yKbmyDVXpSWrTSTXqDBGoA@mail.gmail.com>
	<CA+9eiLvT9JN08hMjN6mH_-WHvB4h5QeAKr7QMSvpz-FmrQGUhQ@mail.gmail.com>
	<CAF+90c_RDzXaLHuEHgZ9Bn6V-MnrnK4zA1PF+Rh_ijWpB4HrMA@mail.gmail.com>
	<CA+9eiLuDxRRXe7HWzqg7dNSiDzaPANk0sENMvr6y-1WVvrcLjQ@mail.gmail.com>
	<CA+9eiLs67UBr7DZ2QmUWSijih7bOPhHbqSu=gvVAPphQ5G-APQ@mail.gmail.com>
Date: Wed, 18 Feb 2015 15:44:11 +0100
Message-ID: <CAF+90c9hCf=85T9kZZViSPt=J+NbcGSv+z252ON20CSjo=RKPQ@mail.gmail.com>
To: Dmitry Stogov <dmitry@zend.com>
Cc: Joe Watkins <pthreads@pthreads.org>, Crypto Compress <cryptocompress@googlemail.com>, 
	PHP Developers Mailing List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=047d7b66f6db48d31c050f5dda6d
Subject: Re: [PHP-DEV] RFC: Expectations
From: nikita.ppv@gmail.com (Nikita Popov)

--047d7b66f6db48d31c050f5dda6d
Content-Type: text/plain; charset=UTF-8

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
>
> 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.

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".

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.

Nikita

--047d7b66f6db48d31c050f5dda6d--