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

[RFC] Closure self-reference #11118

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

KapitanOczywisty
Copy link

@KapitanOczywisty KapitanOczywisty commented Apr 22, 2023

Implementation of https://wiki.php.net/rfc/closure_self_reference

Todo:

  • Plug leak
  • Fix segfault when calling $fn->call($class, 3);
  • Fix opcache
  • Arrow functions
  • Check if ZEND_BIND_SELF_REFERENCE is needed somewhere in opcache
    • May be needed in Zend/Optimizer/sccp.c and/or Zend/Optimizer/zend_dfg.c - not sure
  • More testing

Notes:

  • _zend_ast_decl->childs had to be extended to fit this feature
    maybe should be merged with static binding AST?
  • as $fn is placed before use(...)
    might be sensible to push as after use or even after return type (though latter would look confusing)
    function(int $a) as $fn use($b) : int {} vs
    function(int $a) use($b) as $fn : int {} vs
    function(int $a) use($b) : int as $fn {}

@KapitanOczywisty KapitanOczywisty changed the title Closure self-reference [RFC] Closure self-reference Apr 22, 2023
@Danack
Copy link
Contributor

Danack commented Apr 22, 2023

@mvorisek the internals list is where alternatives to RFCs are discussed. Pull-requests are only for technical discussion of the PR.

@KapitanOczywisty KapitanOczywisty force-pushed the closure_self_reference branch 2 times, most recently from 33b164d to 5a6b033 Compare April 23, 2023 01:10
@KapitanOczywisty KapitanOczywisty marked this pull request as ready for review April 23, 2023 16:01
@dstogov
Copy link
Member

dstogov commented Apr 24, 2023

Just a quick idea...
May be it's better to use some predefined name e.g. __CLOSURE__.
From the implementation point of view, it should reference EX(func).
This way it should be possible to capture it's value into a variable (by value) or call directly with minimal overhead.

$fibonacci = function (int $n) {
    if ($n === 0) return 0;
    if ($n === 1) return 1;
    return __CLOSURE__($n-1) + __CLOSURE__($n-2);
};

I don't tell this proposal is bad and I didn't think about all consequences of my idea.

@KapitanOczywisty
Copy link
Author

KapitanOczywisty commented Apr 24, 2023

@dstogov I was thinking about Closure::current, now I know that both of these would require change in Closure::call - fake closure would have to be removed or closure recreated on __CLOSURE__ (then __CLOSURE__ !== __CLOSURE__ ?) or parser would search for __CLOSURE__ in function body (like arrow fn auto-capture). This would be also first magic constant not returning scalar type.

as $fn could also be moved to use() e.g. use(fn as $fn, $a, $b). We'll see what discussion on internals brings.

@medabkari
Copy link

Any news regarding this RFC?

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.

None yet

5 participants