Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:77011
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.216.175 as permitted sender)
MIME-Version: 1.0
Sender: are.you.winning@gmail.com
In-Reply-To: <A94D26CE-48FE-4642-94BF-330D4A43D925@ajf.me>
References: <E4EDB8A3-20DD-4DB9-AEB8-581B42222628@ajf.me>
	<CABBJUpedHeCArT+8YqUv906vQSbWJ_hvcjgbg-J5CNYZQhyHUg@mail.gmail.com>
	<A94D26CE-48FE-4642-94BF-330D4A43D925@ajf.me>
Date: Mon, 1 Sep 2014 17:29:32 +0100
Message-ID: <CAGAGxbYG_0r1Kc5dY0AqY-c3Ng0pg-DZWoXdkQN4kBxZd2y7CA@mail.gmail.com>
To: Andrea Faulds <ajf@ajf.me>
Cc: Xinchen Hui <laruence@php.net>, PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Make zend_parse_parameters emit E_RECOVERABLE_ERROR
From: cw@daverandom.com (Chris Wright)

On 1 September 2014 17:18, Andrea Faulds <ajf@ajf.me> wrote:
>
> On 1 Sep 2014, at 04:56, Xinchen Hui <laruence@php.net> wrote:
>
>> On Mon, Sep 1, 2014 at 6:01 AM, Andrea Faulds <ajf@ajf.me> wrote:
>>> Good evening,
>>>
>>> Here=E2=80=99s a suggestion: Why don=E2=80=99t we make zend_parse_param=
eters emit
>>
>> ""Why not" is usually not a very good reason for a change in the languag=
e
>> syntax. " =E2=80=94 Stas
>
> The reason isn=E2=80=99t =E2=80=9Cwhy not=E2=80=9D, the reason is to impr=
ove consistency and make programs safer. This makes zpp consistent with use=
rland type hints (less user confusion), and also means programs can failsaf=
e rather than trundle on and do something stupid.
>
>>> E_RECOVERABLE_ERROR on failure, rather than just emitting E_WARNING and
>>> returning FAILURE (causing the caller to typically bail out and return =
NULL)?
>>> This would bring it into line with userland type hints, which also caus=
e such
>>> errors. It might also cause errors to be caught sooner, as just returni=
ng NULL
>>> can cause cryptic problems down the line. It=E2=80=99s worth noting tha=
t
>>> zend_parse_parameters is very tolerant in terms of what parameters you =
can pass
>>> it and is unlikely to error unless you do something really weird such a=
s
>>> passing an array to strlen().
>> PHP is a looser type language... such things might be happened. and
>> internal functions always check their arguments.
>
> Certain types of type shift are uncommon. zpp is extremely tolerant, as i=
t should be in a weakly-typed language, so all of these would work:
>
>     takes_int(1);
>     takes_int(1.0);
>     takes_int(=E2=80=9C     123abc.7=E2=80=9D);
>     takes_int(new StdClass);
>     takes_int(true);
>
> It=E2=80=99s only in quite rare cases that zpp will actually fail and emi=
t an error, and in these cases you probably don=E2=80=99t want your applica=
tion to keep going. In the event you did, you could always make your error =
handler skip these errors. If nikic=E2=80=99s exceptions in the engine RFC =
is revived, you could even do this:
>
>     try {
>         takes_int($something);
>     } catch (InvalidArgumentException $e) {
>         // Handle gracefully
>     }
>
> Which would be more robust than checking for =3D=3D=3D NULL, as some func=
tions return NULL for other reasons and this won=E2=80=99t spit out an E_WA=
RNING.

It's also worth noting that the "return NULL on zpp failure"
convention is not followed to the letter, I have seen places that
RETURN_FALSE - I can't remember exactly where I have seen this but I
will dig a ref out if anyone wants it.

> To help BC, we could even make it do the old thing if @ is being used to =
silence it.

Don't like this

>> this will make the PHP app more robust.
>
> I think applications are more robust if they fail early rather than plodd=
ing along because the programmer forgot an error check, and hence corruptin=
g data or worse.

+1

>>> I doubt this would affect backwards compatibility
>>> much, unless your application relies on a silenced warning (which is po=
ssible,
>>> but discouraged behaviour), and it=E2=80=99s the type of BC break that =
would be really
>>> obvious (your application errors and stops execution).
>> that will be a nightmare to stop people upgrade their PHP.
>
> I doubt it. It=E2=80=99s not something you=E2=80=99re likely to be relyin=
g on (it=E2=80=99s a failure case), and if it is, you can use an error hand=
ler to skip the error and get your old behaviour.
>
> Thanks.
> --
> Andrea Faulds
> http://ajf.me/

I'm roughly -0.5 on this proposal overall. I'd like this to be easily
handleable in userland, but I don't like E_RECOVERABLE_ERROR (in
general, not just here). I'd very much support this if it used
exceptions, though.

Thanks, Chris