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

Pipe operator #5425

Closed
wants to merge 1 commit into from
Closed

Pipe operator #5425

wants to merge 1 commit into from

Conversation

Crell
Copy link
Contributor

@Crell Crell commented Apr 20, 2020

Pipe operator RFC.

At Levi Morison's recommendation, this is implemented purely in the parser layer. The new sigil desugars automatically to the equivalent "inside out" syntax, which means basically all error handling is already written automatically.

Zend/zend_language_parser.y Outdated Show resolved Hide resolved
@kocsismate kocsismate added the RFC label Apr 21, 2020
Zend/zend_compile.h Outdated Show resolved Hide resolved
@guss77
Copy link

guss77 commented Jun 28, 2020

Regarding your "comparison with other languages" in https://wiki.php.net/rfc/pipe-operator-v2, the Ruby equivalent is actually the kernel#then method that is also introduced in Ruby 2.6 (though just its name - the implementation is from earlier versions), so your "getPromotions()" example in the RFC will be implemented in Ruby 2.6 like this:

promotions = getCurrentUser
  .then { |user| getShoppingList(user) }
  .then { |list| mostExpensiveItem(list, 'exclude' => 'onSale') }
  .then { |item| getPromotions(item, holiday) }

@dev-loki
Copy link

As there was no comment for two month and from the rfc I can't really see if and where's the discussion:
What's the status? :)

@morrisonlevi
Copy link
Contributor

morrisonlevi commented Aug 25, 2020

As a tweaked example from the RFC, would you want this code in your codebase?

$result = "Hello World"
    |> 'htmlentities'
    |> 'explode'
    |> fn($x) => array_map('strtoupper', $x)
    |> fn($x) => array_filter($x, fn($v) => $v != 'O');

It would be better if we could use actual symbols instead of strings, and if we could bind directly to a parameter. Syntax subject to change, but something like this where $$ indicates where the piped value will go:

$result = "Hello World"
    |> htmlentities(...)
    |> explode(...)
    |> array_map(strtoupper(...), $$)
    |> array_filter($$, fn($v) => $v != 'O');

The feeling was that for |> to not do harm to existing code we actually do need partial function application, or some other way of making type-safe, non-string callables, or else we're actually going to inflict some harm on code bases because they have few other viable options.

An implementation of partial function application, which would create closures from things like htmlentities(...) and array_filter($$, fn ($v) => $v != 'O'), has not materialized.


Aside: I think it would be even better if we could just use the function names directly, but this would be a major compatibility change and would be much harder to convince internals to accept:

$result = "Hello World"
    |> htmlentities
    |> explode
    |> array_map(strtoupper, $$)
    |> array_filter($$, fn($v) => $v != 'O');

Today a bare htmlentities would cause the engine to look up a constant, not a function. To avoid this issue we'd have to put functions and constants in the same symbol table, which is where the BC break would be. Or we'd have to make special rules around the pipe operator; this would be unlikely to pass as special rules are generally bad in the grammar.

@guss77
Copy link

guss77 commented Aug 25, 2020

If I may hazard a comment, I would prefer:

  • no string callables (at least not as the only or recommended option
  • no bare function names
  • being able to name the piped value, and possibly type hinting it

How about '|' [[<type>] $<name>] '>' as the pipe grammar?

$result = "Hello World"
    |> htmlentities($$)
    |> explode("", $$)
    |array $words> array_map(fn($w) => strtoupper($w), $words)
    |array $caps> array_filter($caps, fn($v) => $v != 'O');

@Crell
Copy link
Contributor Author

Crell commented Aug 25, 2020

At present, there is some on again, off again work on partial function application. If that passes, it would largely resolve the pushback this got. At that point I intend to revive this RFC.

Otherwise, "What Levi said."

@dandrei
Copy link

dandrei commented Sep 21, 2020

Hi @guss77, I like your ideas.

  • no string callables (at least not as the only or recommended option)
  • being able to name the piped value, and possibly type hinting it

Yes, 100%.

  • no bare function names

Why not? Unfortunately there's no language support yet for passing functions around like this, but the following code samples look very clean to me:

$len = 'Hello world' |> strlen;
$size = ['d', 'r', 'a', 'c', 'u', 'l', 'a'] |> count;

@Crell
Copy link
Contributor Author

Crell commented Sep 21, 2020

@dandrei As Levi said above, just writing count on its own would be interpreted by the engine as a constant, not a function name, and changing that is annoyingly hard and BC breaking. Writing it that way would be ideal, but it's also not feasible.

@dandrei
Copy link

dandrei commented Sep 21, 2020

Thanks for the reply, @Crell, you're right about function names by themselves looking like constants to the engine.
An anonymous function assigned to a variable would still work, no?

$count = fn($a) => count($a);
$size = ['d', 'r', 'a', 'c', 'u', 'l', 'a'] |> $count;

@guss77
Copy link

guss77 commented Sep 21, 2020

@dandrei :

  • no bare function names

Why not? Unfortunately there's no language support yet for passing functions around like this, but the following code samples look very clean to me:

$size = ['d', 'r', 'a', 'c', 'u', 'l', 'a'] |> count;

Because it looks ambiguous and confusing. Java uses "method references" like so: ClassName::methodName which I would actually be OK with in PHP as well as it is very clear what is going on - the target callable is easy to identify and figure out its signature, but I think this syntax is foreign and it is a good idea to stay away from it as well.

I think the grammar should simply mirror anonymous callables, just with more optional parts.

An anonymous function looks like this:

argument := [type] '$' name
anon-func := 'fn' '(' [ argument [...] ] ')' '=>' statement

Pipe operation can look like this:

'|' ( '$$' | argument ) '|' '=>' statement

@Crell
Copy link
Contributor Author

Crell commented Sep 21, 2020

I specifically do not want to include any special callable syntax for pipe; they should support whatever callable syntax PHP supports. As Levi notes above, we're hoping to get partial application into 8.1 which would fulfill the goal of a simpler callable syntax, which would also make pipe cleaner.

Until that happens there's nothing else to discuss on this thread.

Comment on lines +1119 to +1120
| expr T_PIPE expr
{ $$ = zend_ast_create(ZEND_AST_CALL, $3, zend_ast_create_list(1, ZEND_AST_ARG_LIST, $1) ); $$->attr = ZEND_CALL_SYNTAX_PIPE; }
Copy link
Contributor

Choose a reason for hiding this comment

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

@nikic Is there something we could/should do with CG(zend_lineno) here? I don't know how the global is really used, so I don't know if this would have any positive impact, or if it should be elsewhere on the line (and I didn't try compiling this):

	|	expr T_PIPE { $<num>$ = CG(zend_lineno); } expr
			{ $$ = zend_ast_create(ZEND_AST_CALL, $4, zend_ast_create_list(1, ZEND_AST_ARG_LIST, $1) ); $$->attr = ZEND_CALL_SYNTAX_PIPE; }

Was trying to think of how we can make it as helpful as possible for debuggers, profilers, etc to have accurate line numbers.

@krakjoe
Copy link
Member

krakjoe commented Jul 20, 2021

Closing, superseded, then declined.

@krakjoe krakjoe closed this Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.