Re: Make zend_parse_parameters emit E_RECOVERABLE_ERROR

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’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