Skip to content

Conversation

@iluuu1994
Copy link
Member

@Crell Please verify.

#1 {main}

#0 %s(%d): {closure:%s:%d}(1)
#1 {main}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite know how to read the closure reference properly. Does the (1) in the second one indicate a nesting problem?

Copy link
Member Author

@iluuu1994 iluuu1994 Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 is a parameter, because print evaluates to 1. https://3v4l.org/0RcBb

@iluuu1994 iluuu1994 requested a review from derickr August 21, 2025 15:31
Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems correct as a stop-gap measure.

@TimWolla TimWolla requested a review from a team August 24, 2025 15:49
Copy link
Member

@edorian edorian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RM approval: 👍

Thank you for sorting this out. Please make a note in NEWS (if you think it's appropriate) so we can tell people about the change in behavior.

@DanielEScherzer
Copy link
Member

RM approval: 👍

Thank you for sorting this out. Please make a note in NEWS (if you think it's appropriate) so we can tell people about the change in behavior.

Yes, definitely please include in NEWS - it'll be removed as part of combining the entries for 8.5.0 (per https://github.com/php/php-src/blob/1cbaa6080758237fc6a292fd0d372ae309ac3b7a/docs/release-process.md#preparing-for-the-initial-stable-version-php-xy0) but would be useful for those testing out the alphas/betas

@iluuu1994
Copy link
Member Author

@Crell Please confirm whether this is the approach you want to go with.

Copy link
Contributor

@Crell Crell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not ideal, but seems the best option available under the circumstances. Thanks, Ilija!

@iluuu1994 iluuu1994 closed this in 71a3226 Aug 25, 2025
@iluuu1994
Copy link
Member Author

Thanks for the inputs. /cc @nikic I'm guessing PHP-Parser will want a similar patch applied.

@kubawerlos
Copy link
Contributor

This makes the examples in RFC invalid. Any chance to update them?

@Crell
Copy link
Contributor

Crell commented Aug 28, 2025

I have no idea what standard procedure is here. I can update the RFC if that's appropriate, but I'm not sure if that's appropriate.

@iluuu1994
Copy link
Member Author

@Crell Given this was discussed publicly I think adjusting the examples is appropriate. For the sake of transparency you may add it to a change log at the top of the page, with a link to the discussion.

@TimWolla
Copy link
Member

I have added an Errata section to my RFCs more than once. Last time https://wiki.php.net/rfc/marking_return_value_as_important#errata

@iluuu1994
Copy link
Member Author

I'd still change the example. It's often the only thing people look at.

@Crell
Copy link
Contributor

Crell commented Aug 28, 2025

I have added an Errata section to the RFC, referencing this issue, and updated the examples accordingly:

https://wiki.php.net/rfc/pipe-operator-v3#errata

@ondrejmirtes
Copy link
Contributor

Hi, is this really necessary when the parentheses do not add any value? For example if the arrow function is the last one in the chain:

$result = $input |> $foo->doFoo(...) |> fn ($x) => $foo->doBar($x);

Adding parentheses around the arrow function is now required but does not disambiguate anything.

@bwoebi
Copy link
Member

bwoebi commented Oct 24, 2025

We could work around this by scanning the AST of the fn body, to check whether it contains |>, but ... $input |> fn ($x) => ($x + 1 |> $foo->doFoo(...)); and $input |> fn ($x) => $x + 1 |> $foo->doFoo(...); for example are not distinguishable from the AST, even though the former would be fine ("last in chain").
There are such caveats which make it annoying; one simple rule "always parens-wrap it" is clearer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants