Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken Function: array_walk_recursive (and more likely others) when using argument reference in callback function #621

Closed
bnowak opened this issue Feb 17, 2025 · 5 comments
Labels
bug Something isn't working

Comments

@bnowak
Copy link

bnowak commented Feb 17, 2025

Function URL

https://www.php.net/manual/en/function.array-walk-recursive.php

PHP Version

8.4

Safe Version

3.0.0

Description

Hello,

I noticed that passing argument by reference in array_walk_recursive doesn't work correctly with PHP 8.4 (like it worked with PHP 8.3).

Example:

$data = [
    ['foo', 'far'],
    ['bar', 'baz'],
];
array_walk_recursive($data, static function (&$item) {
    $item = 111;
});

var_dump($data); // it's still original [['foo', 'far'], ['bar', 'baz']]

# using native PHP array_walk_recursive directly gives expected [[111, 111], [111, 111]] overwritten by reference values

The reason is probably because different way of wrapping it between PHP versions:

# /generated/8.3/array.php
function array_walk_recursive(&$array, callable $callback, $arg = null): void
{
    error_clear_last();
    if ($arg !== null) {
        $safeResult = \array_walk_recursive($array, $callback, $arg);
    } else {
        $safeResult = \array_walk_recursive($array, $callback);
    }
    if ($safeResult === false) {
        throw ArrayException::createFromPhpError();
    }
}

# /generated/8.4/array.php
function array_walk_recursive()
{
    return \array_walk_recursive(...func_get_args());
}

Using func_get_args() probably doesn't handle reference as expected, so other wrapped functions in this way may be also affected.

@shish shish added the bug Something isn't working label Feb 17, 2025
shish added a commit that referenced this issue Feb 17, 2025
shish added a commit that referenced this issue Feb 17, 2025
@shish
Copy link
Collaborator

shish commented Feb 17, 2025

That seems like a correct diagnosis to me, looks like we'll need to generate full functions even in the no-op case D:

#622 adds this as a test case, now to start working on the actual fix...

@bnowak
Copy link
Author

bnowak commented Feb 17, 2025

Thank you

shish added a commit that referenced this issue Feb 17, 2025
shish added a commit that referenced this issue Feb 17, 2025
The minimalist pass-through stubs we used originally would sometimes behave differently and not actually pass things through as intended D:
shish added a commit that referenced this issue Feb 17, 2025
The minimalist pass-through stubs we used originally would sometimes behave differently and not actually pass things through as intended D:
shish added a commit that referenced this issue Feb 17, 2025
The minimalist pass-through stubs we used originally would sometimes behave differently and not actually pass things through as intended D:
@shish shish closed this as completed in 5c2ce65 Feb 17, 2025
@bnowak
Copy link
Author

bnowak commented Feb 18, 2025

@shish thank you very much for fixing that so quickly, you're the boss 😎

Any chances to release that in near future?

@shish
Copy link
Collaborator

shish commented Feb 18, 2025

3.0.1 is released now :)

@bnowak
Copy link
Author

bnowak commented Feb 18, 2025

Thank you 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants