Re: [PHP-DEV] Make zend_parse_parameters emit E_RECOVERABLE_ERROR

On 1 September 2014 17:18, Andrea Faulds <[email protected]> wrote:
>
> On 1 Sep 2014, at 04:56, Xinchen Hui <[email protected]> wrote:
>
>> On Mon, Sep 1, 2014 at 6:01 AM, Andrea Faulds <[email protected]> wrote:
>>> Good evening,
>>>
>>> Here’s a suggestion: Why don’t we make zend_parse_parameters emit
>>
>> ""Why not" is usually not a very good reason for a change in the language
>> syntax. " — Stas
>
> The reason isn’t “why not”, the reason is to improve consistency and make programs safer. This makes zpp consistent with userland type hints (less user confusion), and also means programs can failsafe 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 cause such
>>> errors. It might also cause errors to be caught sooner, as just returning NULL
>>> can cause cryptic problems down the line. It’s worth noting that
>>> 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 as
>>> 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 it should be in a weakly-typed language, so all of these would work:
>
>     takes_int(1);
>     takes_int(1.0);
>     takes_int(“     123abc.7”);
>     takes_int(new StdClass);
>     takes_int(true);
>
> It’s only in quite rare cases that zpp will actually fail and emit an error, and in these cases you probably don’t want your application to keep going. In the event you did, you could always make your error handler skip these errors. If nikic’s 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 === NULL, as some functions return NULL for other reasons and this won’t spit out an E_WARNING.

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 plodding along because the programmer forgot an error check, and hence corrupting data or worse.

+1

>>> I doubt this would affect backwards compatibility
>>> much, unless your application relies on a silenced warning (which is possible,
>>> but discouraged behaviour), and it’s 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’s not something you’re likely to be relying on (it’s a failure case), and if it is, you can use an error handler 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