On Wed, May 1, 2024, at 9:26 PM, Joshua Rüsweg wrote: > Hi > > On 01.05.24 12:26, Larry Garfield wrote: >> This looks good to me, with one remaining exception, which isn't specific to >> this function but should still be discussed: Always passing the value and >> key to the callback is unsafe, for two reasons. >> >> 1. If the callback is an internal function rather than a user-land one, and >> it has only one argument, it will error confusingly. That makes the current >> implementation incompatible with unary built-in functions. See, for >> instance, https://www.php.net/is_string (and friends) > > I think, that this problem can easily be detected with static analysers. > Currently neither PHPStan [1] nor psalm [2] does detect this issue, but > as the tools already validate the signature (e.g. str_contains is > rejected) this can probably be integrated and might even be considered a > bug. > > The proper fix from PHP's side would be something like the proposal in > https://externals.io/message/122928 (RFC idea: using the void type to > control maximum arity of user-defined functions).
It's not just that it errors confusingly. It's that the design as is would be incompatible with any C-based function. A function that works with anything *except* the standard library seems... problematic. It's possible that, since these new functions are C-based, we could do some extra detection to see if the function is unary or not, and pass only the value if it is. I don't know if that is possible to do performantly in C, but it's definitely more of an option than if they were in user-space. (Again, I don't know if this is the best solution, but it seems like a possible solution.) --Larry Garfield
