php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #74351 array_u* functions are not variadic (as reported by reflection)
Submitted: 2017-03-31 18:15 UTC Modified: 2017-05-01 11:10 UTC
Votes:1
Avg. Score:4.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:1 (100.0%)
From: fabien dot villepinte at gmail dot com Assigned: nikic (profile)
Status: Wont fix Package: Arrays related
PHP Version: 7.1.3 OS:
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: fabien dot villepinte at gmail dot com
New email:
PHP Version: OS:

 

 [2017-03-31 18:15 UTC] 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

Patches

Pull Requests

Pull requests:

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2017-03-31 18:43 UTC] requinix@php.net
-Summary: Wrong reflection on variadic array functions +Summary: array_u* functions are not variadic (as reported by reflection) -Status: Open +Status: Analyzed -Package: Reflection related +Package: Arrays related -PHP Version: Irrelevant +PHP Version: 7.1.3
 [2017-03-31 18:43 UTC] 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 19:45 UTC] andrew dot nester dot dev at gmail dot com
Thanks for reporting! I've just added PR fixing this (at least propose fix)
 [2017-04-10 12:11 UTC] 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-04-10 12:48 UTC] requinix@php.net
-Assigned To: +Assigned To: nikic
 [2017-04-10 14:48 UTC] 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-05-01 11:10 UTC] nikic@php.net
-Status: Analyzed +Status: Wont fix
 [2017-05-01 11:10 UTC] nikic@php.net
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).
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Dec 05 11:01:29 2024 UTC