Looks reasonable at first glance, though exact precedence may need some more thought.

@nikic Thanks for your initial assessment!

I wasn't sure about precedence either. throw in an expression will mostly be useful at the end (?? throw, ?: throw, etc). Since combining it with most other operators is nonsensical (e.g. throw new Exception() + 1) it's hard to decide what precedence is correct.

(throw new Exception()) + 1 is the one that would "work" right now. throw (new Exception() + 1) could make more sense if the RFC userspace operator overloading gets accepted.

Of course, there are many other things to consider.

@dstogov @nikic

As the main authors of the code in question I'm taking the liberty of consulting the two of you.

The following snippet:

var_dump(
    throw new Exception("exception 1"),
    throw new Exception("exception 2")
);

Caused a SEGFAULT in the zend_call_graph.c:

because call->caller_call_opline was a null pointer. a95da8aa955dd652bfbbbfc57e896a302fa9ef69 fixes the SEGFAULT but I don't know enough about the optimizer to know if this is an appropriate fix.

@iluuu1994 Also reproduces with

<?php
var_dump(
    exit("exception 1"),
    exit("exception 2")
);

so we should fix this independently.

@iluuu1994 Should be fixed with 34f1266 / 2e8db5d. I don't want to put in a fake value, we have few enough uses that we can just check for NULL.

@nikic Ok, I'll rebase the branch and check if it works.

Looks good, thanks @nikic!

@iluuu1994 Take a look at https://bugs.php.net/bug.php?id=67184 :)

@carusogabriel I'm planning to start the voting on this RFC next week. If it passes I can make another RFC for the remaining keywords 🙂

It seems OPcache (the optimizer?) needs to be adjusted. Currently,

<?php

try {
    throw new Exception(throw new Exception('foo'));
} catch (Exception $ex) {}

leaks memory.

Thanks @cmb69. I tracked it down to this:

152 bytes in 1 blocks are definitely lost in loss record 23 of 27
   at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x5F60EA: __zend_malloc (zend_alloc.c:2976)
   by 0x5F504B: _malloc_custom (zend_alloc.c:2417)
   by 0x5F5179: _emalloc (zend_alloc.c:2536)
   by 0x67B6E5: zend_objects_new (zend_objects.c:196)
   by 0x658987: zend_default_exception_new_ex (zend_exceptions.c:212)
   by 0x658C8D: zend_default_exception_new (zend_exceptions.c:249)
   by 0x632B8B: _object_and_properties_init (zend_API.c:1308)
   by 0x632BFC: object_init_ex (zend_API.c:1322)
   by 0x6AB3A9: ZEND_NEW_SPEC_CONST_UNUSED_HANDLER (zend_vm_execute.h:8858)
   by 0x6FF342: execute_ex (zend_vm_execute.h:52808)
   by 0x702889: zend_execute (zend_vm_execute.h:56143)

(rebased onto master)

I'll see if I can find out what's going on tomorrow.

Is the intention behind removing the ; because using ; means it's a statement, instead of an expression? T_THROW expr ';'

@dragoonis

Expressions evaluate to some value while statements do not. throw is somewhat special in that it can never actually evaluate to anything which is called a bottom type.

In type theory, a theory within mathematical logic, the bottom type is the type that has no values.

https://en.wikipedia.org/wiki/Bottom_type

The main reason for doing so is that throw can be used in places where it previously couldn't have. These are described in the RFC.

@iluuu1994 thanks for explaining.

Here are the unoptimized opcodes:

$_main:
     ; (lines=11, args=0, vars=1, tmps=4)
     ; (before optimizer)
     ; /tmp/php-src/Zend/tests/throw/001.php:1-124
     ; return  [] RANGE[0..0]
0000 V1 = NEW 1 string("Exception")
0001 V2 = NEW 1 string("Exception")
0002 SEND_VAL_EX string("foo") 1
0003 DO_FCALL
0004 THROW V2
0005 SEND_VAL_EX bool(true) 1
0006 DO_FCALL
0007 THROW V1
0008 JMP 0010
0009 CV0($e) = CATCH string("Exception")
0010 RETURN int(1)
LIVE RANGES:
     1: 0001 - 0007 (new)
     2: 0002 - 0004 (new)
EXCEPTION TABLE:
     0000, 0009, -, -

vs the optimized ones:

$_main:
     ; (lines=7, args=0, vars=1, tmps=1)
     ; (after optimizer)
     ; /tmp/php-src/Zend/tests/throw/001.php:1-124
0000 V1 = NEW 1 string("Exception")
0001 V1 = NEW 1 string("Exception")
0002 SEND_VAL_EX string("foo") 1
0003 DO_FCALL
0004 THROW V1
0005 CV0($e) = CATCH string("Exception")
0006 RETURN int(1)
LIVE RANGES:
     1: 0002 - 0004 (new)
EXCEPTION TABLE:
     0000, 0005, -, -

So it looks like it removes everything between the first THROW and the CATCH instruction, including the DO_FCALL but not the NEW instruction. I'm not familiar with the optimizer at all so it might take me a while to get to the bottom of this.

It looks like exit suffers from the same problem.

var_dump(exit());

Unoptimized:

$_main:
     ; (lines=5, args=0, vars=0, tmps=1)
     ; (before optimizer)
     ; /tmp/php-src/Zend/tests/throw/001.php:1-126
     ; return  [] RANGE[0..0]
0000 INIT_FCALL 1 96 string("var_dump")
0001 EXIT
0002 SEND_VAL bool(true) 1
0003 DO_ICALL
0004 RETURN int(1)

Optimized:

$_main:
     ; (lines=2, args=0, vars=0, tmps=0)
     ; (after optimizer)
     ; /tmp/php-src/Zend/tests/throw/001.php:1-126
0000 INIT_FCALL 1 96 string("var_dump")
0001 EXIT

The INIT_FCALL instruction isn't removed but probably should be.

This happens in the CFG optimization pass (pass 5).

As discussed in #5358 the memory leak is related to the live ranges, not INIT_FCALL. Help from somebody experienced with the optimizer would be greatly appreciated!

Thanks @nikic. I'll try to get more familiar with the whole memory management over the next couple of days. I doubt I can come up with a good solution though if you can't. 😆 Should I move the RFC to "Implemented" or wait until we've fully resolved these issues?

Shall we close https://bugs.php.net/bug.php?id=67184? I believe that one is now fixed, right?

Good catch, @carusogabriel. I've closed that ticket.

@iluuu1994 I started some work on this in master...nikic:throw-leak. It's ugly, but I think it will work.

And yes, feel free to move to the RFC to Implemented.

Big thanks @nikic for the help!