On Thu, Aug 14, 2025, at 21:30, Larry Garfield wrote:
> Hi folks. We have discovered a subtle issue with the pipes implementation
> that needs to be addressed, although we're not entirely sure how.
> Specifically, Derick found it while trying to ensure Xdebug plays nice with
> pipes.
>
> The problem has to do with the operator precedence of short closures vs
> pipes. For example:
>
> $result = $arr
> |> fn($x) => array_map(strtoupper(...), $x)
> |> fn($x) => array_filter($x, fn($v) => $v != 'O');
>
> It's logical to assume that would be parsed as
>
> $result = $arr
> |> (fn($x) => array_map(strtoupper(...), $x))
> |> (fn($x) => array_filter($x, fn($v) => $v != 'O'));
>
> Which then compiles into, essentially:
>
> $temp = (fn($x) => array_map(strtoupper(...), $x))($arr);
> $temp = (fn($x) => array_filter($x, fn($v) => $v != 'O'))($temp);
> $result = $temp;
>
> Or
>
> $result = (fn($x) => array_filter($x, fn($v) => $v != 'O'))((fn($x) =>
> array_map(strtoupper(...), $x))($arr));
>
> (depending on how you want to visualize it.)
>
> That was the intent of the RFC.
>
> However, because short closures are "greedy" and assume anything before the
> next semi-colon is part of the closure body, it's actually getting parsed
> like this:
>
> $result = $arr
> |> fn($x) => (
> array_map(strtoupper(...), $x)
> |> fn($x) => (
> array_filter($x, fn($v) => $v != 'O')
> )
> )
> ;
>
> Which would compile into something like:
>
> $result = (fn($x) => (fn($x) => array_filter($x, fn($v) => $v !=
> 'O'))(array_map(strtoupper(...), $x)))($arr);
>
> Which is not the intent.
>
> Humorously, if all the functions and closures involved are pure, this parsing
> difference *usually* doesn't matter. The result is still computed as
> intended. That's why it wasn't caught during earlier reviews or by automated
> tests. However, there are cases where it matters. For example:
>
> 42
> |> fn($x) => $x < 42
> |> fn($x) => var_dump($x);
>
> One would expect that to evaluate to false in the first segment, then
> var_dump() false. But it actually var_dump()s 42, because it gets
> interpreted as (42 |> fn($x) => var_dump($x)) first.
>
> The incorrect wrapping also makes debugging vastly harder, even if the
> computed result is the same, as the expression is broken up "wrong" into
> multiple nested closures, stack traces are different than one would expect,
> etc.
>
> The challenge is conflicting requirements. Closures have extremely low
> precedence right now, specifically so that they will grab everything that
> comes after them as a single expression. However, we also want pipes to
> allow a step to be a closure; that would typically mean making pipe bind even
> lower than closures, but that's not viable because it would result in
>
> $result = 'x' |> fn ($x) => strtoupper($x)
>
> being interpreted as
>
> ($result = 'x') |> (fn ($x) => strtoupper($x))
>
> Which would be rather pointless.
>
> So far, the best suggestion that's been put forward (though we've not tried
> implementing it yet) is to disallow a pipe inside a short-closure body,
> unless the body is surrounded by (). So this:
>
> fn($x) => $x
> |> fn($x) => array_map(strtoupper(...), $x)
> |> fn($x) => array_filter($x, fn($v) => $v != 'O');
>
> Today, that would run somewhat by accident, as the outer-most closure would
> claim everything after it. With the new approach, it would be interpreted as
> passing `fn($x) => $x` as the argument to the first pipe segment, which would
> then be mapped over, which would fail. You'd instead need to do this:
>
> fn($x) => ($x
> |> (fn($x) => array_map(strtoupper(...), $x))
> |> (fn($x) => array_filter($x, fn($v) => $v != 'O'))
> );
>
> Which is not wonderful, but it's not too bad, either. That's probably the
> only case where pipes inside a short-closure body would be useful anyway.
> And if PFA (https://wiki.php.net/rfc/partial_function_application_v2) and
> similar closure improvements pass, it will greatly reduce the need for mixing
> short closures and pipes together in either precedence, so it won't come up
> very often.
>
> There are a few other operators that bind lower than pipe (see
> https://github.com/php/php-src/blob/fd8dfe1bfda62a3bd9dd1ff7c0577da75db02fcf/Zend/zend_language_parser.y#L56-L73),
> which would therefore need wrapping parentheses. For many of them we do
> want them to be lower than pipe, so just moving pipe's priority down isn't
> viable. However, most of those are unlikely to be used inside a pipe segment,
> so are less likely to come up. The most likely would be a bail-out exception:
>
> $value = null;
> $value
> |> fn ($x) => $x ?? throw new Exception('Value may not be null')
> |> fn ($x) => var_dump($x);
>
> Which would currently be interpreted something like:
>
> $c = function ($x) {
> $c = function ($x) {
> return var_dump($x);
> };
> return $x ?? throw $c(new Exception('Value may not be null'));
> };
> $c(null);
>
> This would not throw the exception as expected, unless parentheses are added.
> It would var_dump() an exception and then try to throw the return of
> varl_dump(), which would fatal.
>
> RM approval to address this during the 8.5 beta phase has been given, but we
> still want to have some discussion to make sure we have a good solution.
>
> So, the question:
>
> 1. Does this seem like a good solution, or is there a problem we've not
> spotted yet?
> 2. Does anyone have a better solution to suggest?
>
> Thanks to Derick, Ilija, and Tim for tracking down this annoying edge case.
>
> --
> Larry Garfield
> [email protected]
>
Hi Larry,
What would happen to this:
$x = fn($y) => $y |> strtoupper(…) |> var_dump(…);
with this change? I would expect $x to be a chain — which is what it currently
is. But if I understand correctly, it will become:
$x = (fn($y) => $y) |> strtoupper(...) |> var_dump(...);
which would be interesting... but this that you shared as the current "correct"
way:
> fn($x) => ($x
> |> (fn($x) => array_map(strtoupper(...), $x))
> |> (fn($x) => array_filter($x, fn($v) => $v != 'O'))
> );
This looks correct to me… but I’ve been using pipes since they were merged. So,
I might be biased. I would have written the above like this though:
fn($x) => $x
|> __(array_map(...))(strtoupper(...), __)
|> __(array_filter(...))(__, fn($v) => $v != 'O');
or if I was feeling wordy:
$array_map = __(array_map(...);
$array_filter = __(array_filter(...);
fn($x) => $x => $array_map(strtoupper(...), __) |> $array_filter(__, fn($v) =>
$v != 'O');
This suggested change would completely break that if I'm understanding
correctly. Then PFA would reallow this syntax? I dunno ... changing it makes it
sound inconsistent to me.
FWIW, I like it how it is for writing reusable pipes chains.
PS. The code above is implemented like so:
function __(...$steps): Closure {
$first = array_shift($steps) ?? throw new LogicException();
return function(...$steps) use ($first) {
return function($x) use ($first, $steps) {
foreach($steps as &$val) {
if ($val === __) {
$val = $x;
}
}
return $first(...$steps);
};
};
}
define('__', __(...));
ish.
— Rob
