Re: "finally" handling refactoring (Bug #72213)

On Mon, May 23, 2016 at 1:25 PM, Dmitry Stogov <dmitry@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 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. Nikita