Skip to content

Add short closures / arrow functions#3941

Closed
nikic wants to merge 17 commits into
php:PHP-7.4from
nikic:arrow_function_2
Closed

Add short closures / arrow functions#3941
nikic wants to merge 17 commits into
php:PHP-7.4from
nikic:arrow_function_2

Conversation

@nikic

@nikic nikic commented Mar 13, 2019

Copy link
Copy Markdown
Member

Implementation for https://wiki.php.net/rfc/arrow_functions_v2.

This is based on Levi's old implementation of the same.

@nikic nikic added the RFC label Mar 13, 2019
@carusogabriel

Copy link
Copy Markdown
Contributor

@nikic Will this syntax be allowed:

public class MyClass
{
    private static int $i = 0;

    public static increment(): int => ++self::$i;
}

?

@kunicmarko20

Copy link
Copy Markdown

@carusogabriel If I am correct, this short syntax is for closures only and has nothing to do with methods.

https://github.com/php/php-src/pull/3941/files#diff-7eff82c2c5b45db512a9dc49fb990bb8R170
https://github.com/php/php-src/pull/3941/files#diff-93ad74868f98ff7232ebea00007c8b7fR1270

Based on this you can see that it recognises fn as a Token.

@nikic

nikic commented Mar 13, 2019

Copy link
Copy Markdown
Member Author

@carusogabriel No, this is not supported. It's listed as a possible future extension though: https://wiki.php.net/rfc/arrow_functions_v2#allow_arrow_notation_for_real_functions

@travisfont

travisfont commented Mar 13, 2019

Copy link
Copy Markdown

Arrow functions are ternary operators to functions.

While they are nice and shorten, they can be hard to read at times; considerably to people who aren't used to them which is surprisedly a majority of PHP programmers.

Having them optional sure, but not necessary.

@KalleZ

KalleZ commented Mar 13, 2019

Copy link
Copy Markdown
Member

@tfont Please post feedback on the thread on internals

@SammyK

SammyK commented Apr 21, 2019

Copy link
Copy Markdown
Contributor

This is fantastic - great work folks! :)

I know this PR is still a WIP, but I was playing with it I was able to cause a segfault where I expected a parse error with this little repro:

fn() ==> 42;

This might already be on your radar but just wanted to report it just in case. :)

Backtrace:

php!zend_ast_destroy (/Users/SammyK/Sites/_forks/php-src/Zend/zend_ast.c:789)
php!yydestruct (/Users/SammyK/Sites/_forks/php-src/Zend/zend_language_parser.y:51)
php!zendparse (/Users/SammyK/Sites/_forks/php-src/Zend/zend_language_parser.c:7038)
php!zend_compile (/Users/SammyK/Sites/_forks/php-src/Zend/zend_language_scanner.l:584)
php!compile_file (/Users/SammyK/Sites/_forks/php-src/Zend/zend_language_scanner.l:637)
php!phar_compile_file (/Users/SammyK/Sites/_forks/php-src/ext/phar/phar.c:3347)
php!zend_execute_scripts (/Users/SammyK/Sites/_forks/php-src/Zend/zend.c:1642)
php!php_execute_script (/Users/SammyK/Sites/_forks/php-src/main/main.c:2635)
php!do_cli (/Users/SammyK/Sites/_forks/php-src/sapi/cli/php_cli.c:992)
php!main (/Users/SammyK/Sites/_forks/php-src/sapi/cli/php_cli.c:1384)
libdyld.dylib!start (Unknown Source:0)

@nikic

nikic commented Apr 22, 2019

Copy link
Copy Markdown
Member Author

Note to self: fn needs to be added to the semi-reserved identifier list.

@nikic nikic force-pushed the arrow_function_2 branch from 7188a72 to 1148cbd Compare April 23, 2019 07:56
@nikic

nikic commented Apr 23, 2019

Copy link
Copy Markdown
Member Author

@SammyK Thanks, this is fixed now.

@bwoebi

bwoebi commented Apr 23, 2019

Copy link
Copy Markdown
Member

Can you please add a test for the case where a parameter variable also exists in the parent scope? (ensure that nothing is overwritten, because binding happens only after recv)

e.g.

--FILE--
<?php

$x = 1;
var_dump((fn($x) => $x)(2));
var_dump($x);
var_dump((fn($x = 3) => $x)());
var_dump($x);

?>
--EXPECT--
int(2)
int(1)
int(3)
int(1)

@jbis9051

Copy link
Copy Markdown

@nikic Why even have the fn token? I think it should be similar to JS and just () => .

@theodorejb

Copy link
Copy Markdown
Contributor

@jbis9051 The syntax is discussed at length in the RFC: https://wiki.php.net/rfc/arrow_functions_v2#syntax.

@jbis9051

Copy link
Copy Markdown

@theodorejb I should probably read the entire proposal before asking questions.....sorry

@jmleroy

jmleroy commented Apr 25, 2019

Copy link
Copy Markdown

@theodorejb I cannot find the reason of creating fn instead of reusing function, unless it is for the sake of sparing 6 extra characters.

@jbis9051

Copy link
Copy Markdown

@jmleroy Its shorter. I would even rather f()=>.

@nikic

nikic commented Apr 26, 2019

Copy link
Copy Markdown
Member Author

While brevity is a factor, we also want to have some stronger syntactical distinction between closures that automatically capture scope and those that don't. This especially becomes a problem when you take into account that we may add a block variant of this syntax in the future.

@nikic nikic force-pushed the arrow_function_2 branch from 1148cbd to 5ad4de6 Compare May 2, 2019 08:42
nikic added 7 commits May 2, 2019 10:55
Instead of using an empty use list to distinguish them.

This also fixes the pretty printing to print an arrow function.
There's still a bug here because we're missing the parentheses for
the direct call, but that's an existing issue.
Also rename T_ARROW_FUNCTION to PREC_ARROW_FUNCTION, to make it
obvious that this is not a real token.
@nikic

nikic commented May 2, 2019

Copy link
Copy Markdown
Member Author

Merged as f3e5bbe.

@al-amindev

al-amindev commented Jul 2, 2020

Copy link
Copy Markdown

How to write this one

$factorial = function ($n) use (&$factorial) {
    return $n == 1 ? 1 : $factorial($n - 1) * $n;
};

echo $factorial(5);

arrow syntax?

@theodorejb

Copy link
Copy Markdown
Contributor

@mdalamincse Not all closures can be written as arrow functions, since they only support a single statement and don't capture variables by reference.

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.