Hi,
On 05/23/2016 07:24 PM, Nikita Popov wrote:
On Mon, May 23, 2016 at 1:25 PM, Dmitry Stogov <dmi...@zend.com
<mailto:dmi...@zend.com>> wrote:
Thanks for review.
Both problems should be fixed now
https://gist.github.com/dstogov/0a809891c6a3ac3fac4bd0d9711dd330
Do you see any other problems or a better way to fix this?
Your fix for DISCARD_EXCEPTION does not look right to me. It will
discard all exceptions too early. For example:
function test() {
try {
throw new Exception(1);
} finally {
try {
try {
} finally {
return 42;
}
} finally {
throw new Exception(2);
}
}
}
test();
This will now not chain exception 1 and 2, because exception 1 is
discarded at the return statement.
I thought about this, and I think that the current behavior is right.
If we remove "throw new Exception(2);" the first exception is going to
be discarded at "return 42;".
I think we should do the same independently from the context.
Why do you think Exception(1) should be kept?
I think this should be handled the same way we do the fast_call
dispatch on return, i.e. when we pop the FAST_CALL from the loop var
stack we should replace it with a DISCARD_EXCEPTION and then pop it
after the finally. This should generate all the necessary
DISCARD_EXCEPTION opcodes, and in the right order.
May be I'll commit the existing fix, and you'll try to implement this
idea on top?
Thanks. Dmitry.
Nikita
