Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:93460
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 157.56.111.108 as permitted sender)
To: Nikita Popov <nikita.ppv@gmail.com>
References: <BY2PR0201MB1784296F01E558FF22C48C7ABF4B0@BY2PR0201MB1784.namprd02.prod.outlook.com>
 <CAF+90c_KWrE0PE9p-BWZz9uXaLFUarnbp3h30rN3THz9S7pLjQ@mail.gmail.com>
 <BY2PR0201MB1784A01C040C1332E4B3D7E8BF4E0@BY2PR0201MB1784.namprd02.prod.outlook.com>
 <CAF+90c8NFf2wO4myKtXfUK4GEpUY4ttjS5bePYOKYnYKrNww4g@mail.gmail.com>
CC: Xinchen Hui <xinchen.h@zend.com>, internals <internals@lists.php.net>
Message-ID: <fbe8c464-7e2e-8bc0-5434-0135788c6569@zend.com>
Date: Tue, 24 May 2016 00:36:35 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.0
MIME-Version: 1.0
In-Reply-To: <CAF+90c8NFf2wO4myKtXfUK4GEpUY4ttjS5bePYOKYnYKrNww4g@mail.gmail.com>
Content-Type: multipart/alternative;
	boundary="------------190F36C1C2657A512D7A9459"
SpamDiagnosticOutput: 1:23
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 May 2016 21:36:48.7218 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY2PR0201MB1784
Subject: Re: "finally" handling refactoring (Bug #72213)
From: dmitry@zend.com (Dmitry Stogov)

--------------190F36C1C2657A512D7A9459
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit

Hi,


On 05/23/2016 07:24 PM, Nikita Popov wrote:
> On Mon, May 23, 2016 at 1:25 PM, Dmitry Stogov <dmitry@zend.com 
> <mailto: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 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
>


--------------190F36C1C2657A512D7A9459--