Re: Re: Change to Too Few Args Exception RFC

Dmitry, I've tried to implement this myself, and it seems to work fine, but I end up with some weird (unrelated?) test failures after: https://github.com/php/php-src/compare/master...dshafik:too-few-args-exception Example test failure: ========DIFF======== 001+ Warning: file_get_contents(http://localhost:8964): failed to open stream: Connection refused in /Users/dshafik/src/ php.net/php-src/tests/basic/bug67198.php on line 29 002+ bool(false) 001- string(4) "PASS" 002- string(4) "PASS" 003+ 004+ Warning: file_get_contents(http://localhost:8964): failed to open stream: Connection refused in /Users/dshafik/src/ php.net/php-src/tests/basic/bug67198.php on line 30 005+ bool(false) ========DONE======== FAIL php://input is empty when enable_post_data_reading=Off [tests/basic/bug67198.phpt] and: ========DIFF======== 001+ Warning: Declaration of melt\core\DateTime::createFromFormat($format, $time, ?melt\core\DateTimeZone $timezone = NULL) should be compatible with DateTime::createFromFormat($format, $time, $object = NULL) in /Users/dshafik/src/php.net/php-src/ext/date/tests/bug55407.php on line 7 ========DONE======== List of failed tests: ===================================================================== FAILED TEST SUMMARY --------------------------------------------------------------------- Test posix_getgrgid() function : basic functionality [ext/posix/tests/posix_getgrgid_basic.phpt] Test chdir() function : basic functionality [ext/standard/tests/dir/chdir_basic-win32-mb.phpt] Test chdir() function : usage variations - relative paths [ext/standard/tests/dir/chdir_variation2-win32-mb.phpt] Test getcwd() function : basic functionality [ext/standard/tests/dir/getcwd_basic-win32-mb.phpt] Test readdir() function : usage variations - sub-directories [ext/standard/tests/dir/readdir_variation3-win32-mb.phpt] Test readdir() function : usage variations - different file names [ext/standard/tests/dir/readdir_variation4-win32-mb.phpt] Test readdir() function : usage variations - operate on previously opened directory [ext/standard/tests/dir/readdir_variation6-win32-mb.phpt] Test rewinddir() function : basic functionality [ext/standard/tests/dir/rewinddir_basic-win32-mb.phpt] Test scandir() function : basic functionality [ext/standard/tests/dir/scandir_basic-win32-mb.phpt] Test scandir() function : usage variations - different sorting constants [ext/standard/tests/dir/scandir_variation10-win32-mb.phpt] Test scandir() function : usage variations - different relative paths [ext/standard/tests/dir/scandir_variation4-win32-mb.phpt] Test scandir() function : usage variations - different file names [ext/standard/tests/dir/scandir_variation8-win32-mb.phpt] Test scandir() function : usage variations - different ints as $sorting_order arg [ext/standard/tests/dir/scandir_variation9-win32-mb.phpt] file_get_contents() test using offset parameter out of range [ext/standard/tests/file/file_get_contents_error001.phpt] Test lstat() and stat() functions: usage variations - file opened using w and r mode [ext/standard/tests/file/lstat_stat_variation13.phpt] Check cli_process_title support on Unix [sapi/cli/tests/cli_process_title_unix.phpt] ===================================================================== I can't see how my change has affected this at all; but maybe I missed something? Could you review and see if you spot anything? Thanks, On Sat, Aug 6, 2016 at 12:33 AM, Joe Watkins <pthreads@pthreads.org> wrote: > Morning all, > > Good idea, +1 for just doing it ... > > Cheers > Joe > > On Sat, Aug 6, 2016 at 8:18 AM, Dmitry Stogov <dmitry@zend.com> wrote: > >> I don't see a big problem accepting this. The change is really minor, and >> makes sense. >> >> Dmitry. >> >> On Aug 6, 2016 5:26 AM, Davey Shafik <davey@php.net> wrote: >> Hey all, >> >> I know this is a little late in the process, but it's something I've >> noticed while prepping some content around 7.1. >> >> This RFC: https://wiki.php.net/rfc/too_few_args >> >> Passed, and has been implemented, but I feel that throwing an >> \Error >> exception is a mistake. I think we should another more concrete exception >> class for this error: >> >> \TooFewArgumentsError extends \Error >> >> A use case where this may trivially occur is where you are using argument >> unpacking and the unpacked array is too small. Writing this, just looks bad: >> >> try { >> foo(… $args); >> } catch (\Error $e) { } >> >> compared: >> >> try { >> foo(… $args); >> } catch (\TooFewArgumentsError $e) { } >> >> Thoughts? Dmitry? >> >> Given the tiny change this is, and that is backwards compatible with the >> original RFC, I would like to add this to 7.1 for beta3. >> >> I think I can make this change myself. >> >> - Davey >> >> >> >> >