Bug #74351 [Ana->Wfx]: array_u* functions are not variadic (as reported by reflection)

From: Date: Mon, 01 May 2017 11:11:07 +0000
Subject: Bug #74351 [Ana->Wfx]: array_u* functions are not variadic (as reported by reflection)
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-208884@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=74351&edit=1 ID: 74351 Updated by: nikic@php.net Reported by: fabien dot villepinte at gmail dot com Summary: array_u* functions are not variadic (as reported by reflection) -Status: Analyzed +Status: Wont fix Type: Bug Package: Arrays related PHP Version: 7.1.3 Assigned To: nikic Block user comment: N Private report: N New Comment: Marking this as Won't Fix per the discussion here and on the PR. The tl;dr is that the precise signature of these functions is not expressible in PHP. If we ever support non-trailing variadic arguments officially we may reconsider this, but for now these functions should fall in the class of "using func_get_args() to implement a weird, non-standard signature". I'm not willing to allow an exception for internal functions here, as the reflection-only benefit is rather dubious compared to the headaches this could cause in the future (if, say, someone used such a signature not on a function, but a method, in which case we'd be forced to implement variance checks for such signatures). Previous Comments: ------------------------------------------------------------------------ [2017-04-10 14:48:51] requinix@php.net I don't think we can agree on that, actually. It's like I said earlier and what was said in the PR: while yes, the parameter certainly looks and acts variadic, it isn't actually. Built-in functions should have to follow the same rules as userland functions, and if function array_diff_uassoc(array $array1, array ...$array2, callable $key_compare_func) isn't permitted for us users then it shouldn't be allowed internally either. @nikic also mentioned a larger problem that I suspected might be the case: designs in the engine that assume/require a variadic to be the last parameter. Fixing everything would probably take PHP to being only a couple steps away from supporting the variadic in any position... Anyway, it's not like these functions are the only awkward cases for reflection: - strtok() has a string and a string,string syntax, that being analogous to the weird way C's strtok works - strtr() has a string,string,string and a string,array syntax - implode() has a string,array and an array[,string] syntax ------------------------------------------------------------------------ [2017-04-10 12:11:24] fabien dot villepinte at gmail dot com As far as I understand the discussion ( https://github.com/php/php-src/pull/2446 ) these functions should not be declared as variadic. Are we agree that at least the 2nd parameter is variadic and ReflectionParameter::isVariadic should return true? ------------------------------------------------------------------------ [2017-03-31 19:45:44] andrew dot nester dot dev at gmail dot com Thanks for reporting! I've just added PR fixing this (at least propose fix) ------------------------------------------------------------------------ [2017-03-31 18:43:06] requinix@php.net Reflection is working correctly: those functions are not actually defined internally as being variadic. https://github.com/php/php-src/blob/PHP-7.1.3/ext/standard/basic_functions.c#L534 It's C but the assorted ZEND_BEGIN_ARG_INFOs are fairly self-explanatory. Note the use of ZEND_ARG_INFO for regular parameters and ZEND_ARG_VARIADIC_INFO for variadic parameters. The problem is that they all take a final callback parameter (or two) after the varargs list, and that doesn't fit the rules for how variadics work. What PHP did is akin to the PHP <5.6 style of using func_get_args(), and like a corresponding userland implementation the parameters will not be considered variadic. I'll move this to Analyzed but I'm not sure whether it can/should be fixed. ------------------------------------------------------------------------ [2017-03-31 18:15:08] fabien dot villepinte at gmail dot com Description: ------------ The following functions are variadic but ReflectionFunction::isVariadic and ReflectionParameter::isVariadic return false : array_diff_uassoc array_diff_ukey array_intersect_uassoc array_intersect_ukey array_udiff array_udiff_assoc array_udiff_uassoc array_uintersect array_uintersect_assoc array_uintersect_uassoc The test script looks more complicated than necessary because I wanted to highlight the differencies with HHVM (see : https://3v4l.org/clKBH ). Test script: --------------- <?php $functions = [ 'array_diff_uassoc', 'array_diff_ukey', 'array_intersect_uassoc', 'array_intersect_ukey', 'array_udiff', 'array_udiff_assoc', 'array_udiff_uassoc', 'array_uintersect', 'array_uintersect_assoc', 'array_uintersect_uassoc', ]; foreach ($functions as $func) { $reflFunc = new ReflectionFunction($func); $nbParams = $reflFunc->getNumberOfParameters(); $nbRequiredParams = $reflFunc->getNumberOfRequiredParameters(); printf("$func ($nbParams params, $nbRequiredParams required) %s VARIADIC", $reflFunc->isVariadic() ? "IS" : "IS NOT"); foreach ($reflFunc->getParameters() as $i => $param) { if ($param->isVariadic()) { printf(" - PARAM #%d%s IS VARIADIC", $i + 1, $param->isOptional() ? " (Opt.)" : ""); } } echo PHP_EOL; } Expected result: ---------------- array_diff_uassoc (3 params, 3 required) IS VARIADIC - PARAM #2 IS VARIADIC array_diff_ukey (3 params, 3 required) IS VARIADIC - PARAM #2 IS VARIADIC array_intersect_uassoc (3 params, 3 required) IS VARIADIC - PARAM #2 IS VARIADIC array_intersect_ukey (3 params, 3 required) IS VARIADIC - PARAM #2 IS VARIADIC array_udiff (3 params, 3 required) IS VARIADIC - PARAM #2 IS VARIADIC array_udiff_assoc (3 params, 3 required) IS VARIADIC - PARAM #2 IS VARIADIC array_udiff_uassoc (4 params, 4 required) IS VARIADIC - PARAM #2 IS VARIADIC array_uintersect (3 params, 3 required) IS VARIADIC - PARAM #2 IS VARIADIC array_uintersect_assoc (3 params, 3 required) IS VARIADIC - PARAM #2 IS VARIADIC array_uintersect_uassoc (4 params, 4 required) IS VARIADIC - PARAM #2 IS VARIADIC Actual result: -------------- array_diff_uassoc (3 params, 3 required) IS NOT VARIADIC array_diff_ukey (3 params, 3 required) IS NOT VARIADIC array_intersect_uassoc (3 params, 3 required) IS NOT VARIADIC array_intersect_ukey (3 params, 3 required) IS NOT VARIADIC array_udiff (3 params, 3 required) IS NOT VARIADIC array_udiff_assoc (3 params, 3 required) IS NOT VARIADIC array_udiff_uassoc (4 params, 4 required) IS NOT VARIADIC array_uintersect (3 params, 3 required) IS NOT VARIADIC array_uintersect_assoc (3 params, 3 required) IS NOT VARIADIC array_uintersect_uassoc (4 params, 4 required) IS NOT VARIADIC ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=74351&edit=1

« previous php.bugs (#208884) next »