GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — first class callable conversion by krakjoe · Pull Request #7019 · php/php-src

Conversation

@krakjoe

@krakjoe krakjoe commented May 20, 2021

Copy link
Copy Markdown
Member

Impl for https://wiki.php.net/rfc/first_class_callable_syntax

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cmb69, bwoebi, KennedyTedesco, alherd-by, zlianon, zmitic, dktapps, vanodevium, byan, N-Silbernagel, and 4 more reacted with thumbs up emoji
All reactions
  • 👍 14 reactions
@nikic nikic added the RFC label May 20, 2021
nikic
nikic reviewed May 20, 2021
View reviewed changes
Comment thread Zend/tests/first_class_callable_005.phpt Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Zend/zend_compile.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Zend/zend_closures.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

bwoebi
bwoebi reviewed May 20, 2021
View reviewed changes
Comment thread Zend/zend_closures.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

mvorisek
mvorisek reviewed May 20, 2021
View reviewed changes
Comment thread Zend/tests/first_class_callable_003.phpt Resolved

Uh oh!

There was an error while loading. Please reload this page.

@TysonAndre

TysonAndre commented May 20, 2021

Copy link
Copy Markdown
Contributor

Would it make sense to update zend_undefined_function_helper (etc) to indicate that it is creating a closure from an undefined function (etc) rather than calling the function at the time the error is thrown?
(e.g. check if the next opcode is creating a closure - ZEND_CALLABLE_CONVERT,)

The current implementation:

ZEND_VM_COLD_HELPER(zend_undefined_function_helper, ANY, ANY)
{
	USE_OPLINE
	zval *function_name;

	SAVE_OPLINE();
	function_name = RT_CONSTANT(opline, opline->op2);
	zend_throw_error(NULL, "Call to undefined function %s()", Z_STRVAL_P(function_name));
	HANDLE_EXCEPTION();
}
php > $x = ('missing')(...);

Warning: Uncaught Error: Call to undefined function missing() in php shell code:1
Stack trace:
#0 {main}
  thrown in php shell code on line 1
php > $x = Closure::fromCallable('missing');

Warning: Uncaught TypeError: Failed to create closure from callable: function "missing" not found or invalid function name in php shell code:1
Stack trace:
#0 php shell code(1): Closure::fromCallable('missing')
#1 {main}
  thrown in php shell code on line 1
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

TysonAndre
TysonAndre reviewed May 20, 2021
View reviewed changes
Comment thread Zend/zend_language_parser.y Resolved

Uh oh!

There was an error while loading. Please reload this page.

@nikic

nikic commented May 25, 2021

Copy link
Copy Markdown
Member

@TysonAndre Having a more specific error message would be nice, but it wouldn't be as simple as changing that one place. For example, the error for calling an inaccessible method is generated in the get_method object handler, and I don't think we want to thread information about what kind of "call" it is through there.

Possibly we could make the message more generic so it would work in both situations, for example:

Old: Call to undefined function foobar()
New: Reference to undefined function foobar()

Old: Call to private method Foo::bar() from Baz
New: Reference to private method Foo::bar() from Baz

Though maybe that makes the message in the common case slightly harder to understand.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nikic
nikic force-pushed the first-class-callable-conversion branch from 161b714 to fe21832 Compare June 29, 2021 08:26
@thekid thekid mentioned this pull request Jul 2, 2021
Implement first-class callable syntax xp-framework/compiler#114
Merged
@mvorisek

mvorisek commented Jul 5, 2021

Copy link
Copy Markdown
Contributor

Please improve also dump of Closure to contain the specific method/function name. https://3v4l.org/SjSnR

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nikic nikic added this to the PHP 8.1 milestone Jul 9, 2021
@nikic
nikic force-pushed the first-class-callable-conversion branch from 5635d27 to 3fcece3 Compare July 13, 2021 13:00
@nikic

nikic commented Jul 13, 2021

Copy link
Copy Markdown
Member

I added some more test coverage. Anything still missing?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nikic
nikic force-pushed the first-class-callable-conversion branch from 2336613 to 7a3ac9f Compare July 13, 2021 15:13
krakjoe and others added 16 commits July 13, 2021 18:44
@krakjoe @nikic
first class callable conversion
b335041
@krakjoe @nikic
tests
3116072
@krakjoe @nikic
ws
211f869
@krakjoe @nikic
preserve called scope
02ec61c
@krakjoe @nikic
fix test as suggested, for real this time
92e240f
@krakjoe @nikic
test and fix
ea39a46
@krakjoe @nikic
handle attributes nicely
6a771bb
@nikic
Forbid callable syntax + nullsafe
af43182
@nikic
Test strict types behavior
b004008
@nikic
fix call graph
4dd984c
@nikic
Fix indent
6c036bd
@nikic
Fix assertion handling
d7488c3
Fix acquiring callable to assert() in namespace, and also add
support for (...) to the AST printer.
@nikic
Add additional test coverage
0c433d3
@nikic
Fix optimizer support for first-class callables
180125d
@nikic
Some JIT handling
7ea5139
@nikic
Bail out from tracing JIT
f42744e
I wasn't able to make this work with tracing JIT quickly.
nikic added 2 commits July 13, 2021 18:44
@nikic
Remove assert
4370252
Does not necessarily hold (e.g. exit in call args).
@nikic
Add test I forgot to git add
f3af1d4
@nikic
nikic force-pushed the first-class-callable-conversion branch from 7a3ac9f to f3af1d4 Compare July 13, 2021 16:45
@nikic nikic closed this in d0b09a7 Jul 14, 2021
@cmb69 cmb69 mentioned this pull request May 24, 2022
Awkward error using first-class callable syntax with missing function #8623
Open
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Dec 5, 2022
@jrfnl
✨ PHP 8.1: New PHPCompatibility.Syntax.NewFirstClassCallables sniff
3f2c558
> Closures for callables can now be created using the syntax `myFunc(...)`, which is identical to `Closure::fromCallable('myFunc')`.

Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.core.callable-syntax
* https://wiki.php.net/rfc/first_class_callable_syntax
* php/php-src#7019
* php/php-src@d0b09a7

Includes unit tests.
Includes sniff documentation.
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Dec 5, 2022
@jrfnl
✨ PHP 8.1: New PHPCompatibility.Syntax.NewFirstClassCallables sniff
04ec74c
> Closures for callables can now be created using the syntax `myFunc(...)`, which is identical to `Closure::fromCallable('myFunc')`.

Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.core.callable-syntax
* https://wiki.php.net/rfc/first_class_callable_syntax
* php/php-src#7019
* php/php-src@d0b09a7

Includes unit tests.
Includes sniff documentation.
@jrfnl jrfnl mentioned this pull request Dec 5, 2022
✨ PHP 8.1: New PHPCompatibility.Syntax.NewFirstClassCallables sniff PHPCompatibility/PHPCompatibility#1425
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TysonAndre TysonAndre TysonAndre left review comments
@bwoebi bwoebi bwoebi left review comments
+3 more reviewers
@patrickallaert patrickallaert patrickallaert left review comments
@nikic nikic nikic left review comments
@mvorisek mvorisek mvorisek left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

RFC

Projects

None yet

Milestone

PHP 8.1

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@krakjoe @TysonAndre @nikic @mvorisek @patrickallaert @bwoebi
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.