Skip to content

[RFC] Partial Function Application - #6898

Closed
krakjoe wants to merge 35 commits into
php:masterfrom
krakjoe:partials
Closed

[RFC] Partial Function Application#6898
krakjoe wants to merge 35 commits into
php:masterfrom
krakjoe:partials

Conversation

@krakjoe

@krakjoekrakjoe commented Apr 22, 2021•
edited
Loading

Copy link
Copy Markdown
Member

@krakjoekrakjoe added the RFC label Apr 22, 2021
azjezz
azjezz reviewed Apr 22, 2021
View reviewed changes
Comment threadZend/zend_partial.stub.php Outdated
@krakjoekrakjoe closed this Apr 24, 2021
@krakjoekrakjoe reopened this Apr 24, 2021
@krakjoekrakjoe closed this Apr 26, 2021
@krakjoekrakjoe reopened this Apr 26, 2021
@azjezzazjezz mentioned this pull request Apr 26, 2021
Closed
krakjoe
krakjoe commented Apr 30, 2021
View reviewed changes
Comment threadZend/Optimizer/zend_call_graph.c Outdated
Comment threadZend/zend_compile.c Outdated
Comment threadZend/zend_compile.h Outdated
Comment threadZend/zend_object_handlers.h Outdated
Comment threadZend/zend_opcode.c Outdated
Comment threadZend/zend_vm_def.h Outdated
Comment threadext/reflection/php_reflection.c Outdated
Comment threadext/reflection/php_reflection.c Outdated
Comment threadext/reflection/php_reflection.c Outdated
Comment threadext/reflection/tests/ReflectionExtension_getClasses_basic.phpt Outdated
@nikic

nikic commented May 12, 2021

Copy link
Copy Markdown
Member

I'm missing a test for __call/__callStatic trampolines.

nikic
nikic reviewed May 12, 2021
View reviewed changes
Comment threadZend/tests/partial_function/nested.phpt Outdated
Comment threadZend/tests/partial_function/object.phpt Outdated
Comment threadZend/tests/partial_function/reflection.phpt Outdated
Comment threadZend/tests/partial_function/type-check.phpt Outdated
Comment threadZend/tests/partial_function/variadics-named-nested-overwrite.phpt Outdated
dkulyk
dkulyk reviewed May 13, 2021
View reviewed changes
Comment threadZend/tests/partial_function/named-args.phpt Outdated
nicolas-grekas
nicolas-grekas reviewed May 13, 2021
View reviewed changes
Comment threadZend/tests/partial_function/reflection-resolver.phpt Outdated
POPSuL
POPSuL reviewed May 13, 2021
View reviewed changes
Comment threadZend/tests/partial_function/object-constructor.phpt Outdated
nikic
nikic reviewed May 14, 2021
View reviewed changes
Comment threadZend/zend_ast.h Outdated
Comment threadZend/tests/partial_function/type-check.phpt Outdated
Comment threadZend/zend_closures.c Outdated
Comment threadZend/zend_compile.c Outdated
Comment threadZend/zend_compile.c Outdated
nikic
nikic reviewed May 14, 2021
View reviewed changes
Comment threadZend/zend_compile.c Outdated
Comment threadZend/zend_compile.c Outdated
Comment threadZend/zend_compile.c Outdated
Comment threadZend/zend_compile.c Outdated
Comment threadZend/zend_compile.c Outdated
Comment threadext/reflection/php_reflection.c Outdated
Comment threadZend/zend_vm_def.h Outdated
Comment threadZend/zend_vm_def.h Outdated
Comment threadZend/zend_vm_def.h Outdated
Comment threadZend/zend_vm_def.h Outdated
nikic
nikic reviewed May 14, 2021
View reviewed changes
Comment threadZend/zend_partial.c Outdated
Comment threadZend/zend_partial.c Outdated
Comment threadZend/zend_vm_def.h Outdated
nikic
nikic reviewed May 14, 2021
View reviewed changes
Comment threadZend/zend_partial.c Outdated
Comment threadZend/zend_partial.c Outdated
Comment threadZend/zend_partial.c Outdated
Comment threadZend/zend_partial.c Outdated
Comment threadZend/zend_partial.c Outdated

@nikicnikicMay 14, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So for the new Foo(?) case, we first create an object, then destroy it and then if the partial is invoked, create another object.

This is non-great, though I'm not sure just how non-great it is. In conjunction with https://wiki.php.net/rfc/new_in_initializers it could mean that property default values get instantiated and immediately dropped again (though arguably that's an issue with that RFC).

@krakjoekrakjoeMay 14, 2021

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

If we have space for a flag, we could use IS_OBJ_CONSTRUCTOR_CALLED here and use the initial object ...

Comment threadZend/zend_partial.c Outdated
Comment threadZend/zend_partial.c Outdated
Comment threadZend/zend_partial.c Outdated
@krakjoekrakjoe closed this May 19, 2021
@krakjoekrakjoe reopened this May 28, 2021
@krakjoe
krakjoe marked this pull request as draft May 28, 2021 08:52
@krakjoe
krakjoeforce-pushed the partials branch 2 times, most recently from d264f87 to 61e7f1eCompareMay 29, 2021 16:24
@krakjoe
krakjoe marked this pull request as ready for review May 29, 2021 16:58
@krakjoe
krakjoeforce-pushed the partials branch 3 times, most recently from 11123b8 to 286e16fCompareMay 29, 2021 20:55
@nikic

This comment has been minimized.

Sign in to view
@krakjoe

This comment has been minimized.

Sign in to view
nikic
nikic reviewed Jun 18, 2021
View reviewed changes
Comment threadZend/zend_partial.c Outdated
Comment threadZend/zend_partial.c Outdated
Comment threadZend/zend_partial.c Outdated
Comment threadZend/zend_partial.c Outdated
Comment threadZend/zend_partial.c Outdated
Comment threadZend/zend_partial.c Outdated
Comment threadZend/zend_partial.c Outdated
Comment threadZend/zend_partial.c
Comment threadZend/zend_partial.c Outdated
@krakjoe

This comment has been minimized.

Sign in to view
@nikic

This comment has been minimized.

Sign in to view
@krakjoe

This comment has been minimized.

Sign in to view
@nikic

This comment has been minimized.

Sign in to view
@krakjoe

This comment has been minimized.

Sign in to view
@krakjoe

This comment has been minimized.

Sign in to view
patrickallaert
patrickallaert reviewed Jun 19, 2021
View reviewed changes
Comment threadext/reflection/php_reflection.c
Comment on lines +1326 to +1328
}
ZEND_ASSERT(0);
return -1;

@patrickallaertpatrickallaertJun 19, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Indentation: mix of tabs and spaces.

@thekidthekid mentioned this pull request Jun 27, 2021
Closed
@faizanakram99

faizanakram99 commented Jul 1, 2021

Copy link
Copy Markdown

We lost :(

@krakjoekrakjoe closed this Jul 1, 2021
@POPSuL

POPSuL commented Jul 1, 2021

Copy link
Copy Markdown

What's next? Will there be an alternative from Nikita, or your collaboration?

@nikicnikic mentioned this pull request Jul 13, 2021
Closed
@jurchiks

jurchiks commented Feb 25, 2022

Copy link
Copy Markdown

Damn shame, it looks to me like a very useful feature to have.

@arnaud-lbarnaud-lb mentioned this pull request Jun 20, 2025
Draft
10 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kamil-tekielakamil-tekielakamil-tekiela left review comments
+10 more reviewers
@patrickallaertpatrickallaertpatrickallaert left review comments
@drealecsdrealecsdrealecs left review comments
@nikicnikicnikic left review comments
@nicolas-grekasnicolas-grekasnicolas-grekas left review comments
@CrellCrellCrell left review comments
@dkulykdkulykdkulyk left review comments
@POPSuLPOPSuLPOPSuL left review comments
@francislavoiefrancislavoiefrancislavoie left review comments
@someniatkosomeniatkosomeniatko left review comments
@azjezzazjezzazjezz left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

RFC

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

14 participants

@krakjoe@nikic@faizanakram99@POPSuL@jurchiks@patrickallaert@drealecs@nicolas-grekas@Crell@dkulyk@francislavoie@kamil-tekiela@someniatko@azjezz