On Fri, Aug 19, 2016 at 10:27 PM, Kalle Sommer Nielsen <ka...@php.net> wrote:
> 2016-08-19 15:05 GMT+02:00 Davey Shafik <da...@php.net>: > > Hey Internals, > > > > I'm working on the patch to change the error exception to > > \ArgumentCountError instead of \Error when too few, or too many arguments > > are passed in as discussed a couple of weeks ago. > > > > To be BC with 7.0, this extends \TypeError, which is the exception > > currently thrown when strict_types=1 for both userland and internal > > functions. > > > > I've run into an issue where things are inconsistent for internal > functions. > > > > For example, the array_* functions, like array_diff, still have a warning > > thrown, regardless of strict_types: > > > > php_error_docref(NULL, E_WARNING, "at least %d parameters are required, > %d > > given", req_args, ZEND_NUM_ARGS()); > > This seems more like an oversight than anything else, I guess cases > with variable argument count may have some similar parameter parsing > approaches. I think it should just be changed to the exception and to > respect strict_types. Despite it being a BC break, I don't think that > many will care to notice since its error handling for one and second > of all, we did do that for a lot of things in 7.0, and I would assume > that most of the userland developers expect us to continue converting > into exceptions where reasonable, so the programs will *hopefully* > quickly adapt. I have completed this as best as I can tell. All test failures also exist on master without the patch. https://github.com/php/php-src/pull/2092 The only potential issue is that the format of the warning message has changed slightly from e.g.: -Warning: array_diff(): at least 2 parameters are required, 0 given in %s on line %d +Warning: array_diff() expects at least 2 parameters, 0 given in %s on line %d I suspect there are other functions that are not array functions that emit similar warnings (especially for functions that expect _exact_ number of arguments) that I missed, so if you know any of these, please let me know and I'll update the patch. If everyone is good with this I will merge down to PHP 7.1 for RC1 next week. - Davey
