Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:95363
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain daveyshafik.com from 209.85.218.42 cause and error)
MIME-Version: 1.0
Sender: me@daveyshafik.com
In-Reply-To: <CAJW__o2EEFcgcq5CxbL+1tw-x_9m-AqC+xKVT2ZzOZUuCtL6ug@mail.gmail.com>
References: <CANaX5nfdhKwau8jKBw1ABeLznyOeQ0iEAavwm+6izFevLBQRfg@mail.gmail.com>
 <CAJW__o2EEFcgcq5CxbL+1tw-x_9m-AqC+xKVT2ZzOZUuCtL6ug@mail.gmail.com>
Date: Mon, 22 Aug 2016 16:18:33 +0800
Message-ID: <CANaX5nf5zuZxA_mTbSjvgsAkuufRRpqiEUoWDx=AwR4nabGuUw@mail.gmail.com>
To: Kalle Sommer Nielsen <kalle@php.net>
Cc: "internals@lists.php.net" <internals@lists.php.net>, Dmitry Stogov <dmitry@zend.com>
Content-Type: multipart/alternative; boundary=001a113dc22ab0ad5a053aa4b18e
Subject: Re: [PHP-DEV] Incorrect Argument Count Error
From: davey@php.net (Davey Shafik)

--001a113dc22ab0ad5a053aa4b18e
Content-Type: text/plain; charset=UTF-8

On Fri, Aug 19, 2016 at 10:27 PM, Kalle Sommer Nielsen <kalle@php.net>
wrote:

> 2016-08-19 15:05 GMT+02:00 Davey Shafik <davey@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

--001a113dc22ab0ad5a053aa4b18e--