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

feat: make break, continue and return be an expression #17647

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

Conversation

xepozz
Copy link

@xepozz xepozz commented Jan 31, 2025

Hi!

Long time playing with Kotlin and I like how break, continue and return works there.
Tried to implement such features in PHP. Not sure about how correct implementation, but did almost the same as in 0810fcd

I'll try to create RFC a bit later, but now we can discuss the idea.


Yeah, now you can write not only these:

throw throw throw new Exception();

But also

function test() {
    return return return throw throw throw new Exception();
}

😁

@TimWolla
Copy link
Member

Not sure about how correct implementation,

I recommend to add some tests with objects implementing __destruct() to verify correct garbage collection / to verify that lifetimes work as expected (i.e. objects are destructed at the correct moment).

@iluuu1994
Copy link
Member

iluuu1994 commented Jan 31, 2025

Unfortunately, this comes with multiple technical challenges that are described in https://wiki.php.net/rfc/match_blocks#technical_implications_of_control_statements. There are also some optimizer issues that are not documented there. It's a very hard problem to solve.

@xepozz
Copy link
Author

xepozz commented Jan 31, 2025

Unfortunately, this comes with multiple technical challenges that are described in https://wiki.php.net/rfc/match_blocks#technical_implications_of_control_statements. There are also some optimizer issues that are not documented there. It's a very hard problem to solve.

In kotlin there are jumps, like context points, that control what level you are going to prevent. May it help?

In examples:

function f() {
 while() {
    match ($foo) {
        'bar' => {
            // Returning 'baz' from the *function* (not match)
            // This is ok, because match is a standalone expression
            return 'baz'; // collapse and free `f()` context
            return@match 'baz'; // result of match()
        },
    };
 }
}

@iluuu1994
Copy link
Member

@xepozz Not quite (but it's good to know this exists nonetheless!). You can try this snippet on your branch:

class Foo {}

function test() {
    new Foo() + return;
}

test();

This will, unless I'm badly mistaken, leak memory. You should see a log in the console if you're compiling with --enable-debug.

@xepozz
Copy link
Author

xepozz commented Jan 31, 2025

@xepozz Not quite (but it's good to know this exists nonetheless!). You can try this snippet on your branch:

class Foo {}

function test() {
    new Foo() + return;
}

test();

This will, unless I'm badly mistaken, leak memory. You should see a log in the console if you're compiling with --enable-debug.

Got it


---- EXPECTED OUTPUT

---- ACTUAL OUTPUT
[Fri Jan 31 17:22:56 2025]  Script:  '/Users/xepozz/IdeaProjects/xepozz/php-src/tests/lang/early-return/memory_leaks.php'
/Users/xepozz/IdeaProjects/xepozz/php-src/Zend/zend_objects.c(210) :  Freeing 0x0000000102401fa0 (40 bytes), script=/Users/xepozz/IdeaProjects/xepozz/php-src/tests/lang/early-return/memory_leaks.php
=== Total 1 memory leaks detected ===
---- FAILED

@bwoebi
Copy link
Member

bwoebi commented Jan 31, 2025

I think the main thing which needs to change is basically live temporary cleanup, like it happens whenever an exception is thrown. Possibly FAST_RET mechanisms could be reused, but doing so won't be trivial.
This is not related to the problems which match has, which revolve around keeping the temporary liveness correct across its execution.

But yes, this needs VM support.

I don't know about optimizer, it might indeed violate some assumptions there. Maybe not if FAST_RET is just reused - which optimizer knows to handle.

@iluuu1994
Copy link
Member

iluuu1994 commented Jan 31, 2025

@bwoebi

This is very much the same issue as match. break as an expr is essentially equivalent to match (1) { 1 => { break 2; } }. This comes with three problems:

  1. We may compile jumps out of the current expression, skipping the consumption of live temporaries.
  2. We're not cleaning up started calls (foo(break)).
  3. DCE in the optimizer can break assumptions for live-range analysis by removal of the consuming opcodes (e.g. in case we always return in the expression).

I think the main thing which needs to change is basically live temporary cleanup, like it happens whenever an exception is thrown. Possibly FAST_RET mechanisms could be reused, but doing so won't be trivial.

I have essentially tried that as of a few days ago: master...iluuu1994:php-src:match-blocks-2

This solves problem 1 and 2, but it still turned out to be much more complex that I was hoping, to the point where my original solution (master...iluuu1994:php-src:match-blocks-var-tracking) isn't much worse. It doesn't solve problem 3 yet, which I will still need to have a closer look at.

@iluuu1994
Copy link
Member

Yeah, now you can write not only these:

throw throw throw new Exception();

But also

function test() {
    return return return throw throw throw new Exception();
}

And of course, @xepozz, you should try to come up with actual, useful examples. return return is obviously not useful.

Something that may be is:

while ($cond) {
    ...
    $element = $element->parent() ?? break;
    ...
}

$value = $cond ? $x : return false;

foreach (...) {
    match ($value) {
        ...
        default => continue 2,
    };
}

But tbh, use-cases seem limited. If they are, the complexity of the solution will be hard to justify.

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.

4 participants