Fix first part on liveness analysis of bug https://code.google.com/p/dart/issues/detail?id=9687: a …

Fix first part on liveness analysis of bug https://code.google.com/p/dart/issues/detail?id=9687: a throw in a catch block must have the finally block as a successor.
Committed: https://code.google.com/p/dart/source/detail?r=21278

[ngeoffray, 2013-04-10 14:51:16]

https://codereview.chromium.org/14071003/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/14071003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:3902:
handleInTryStatement();
We do this for return/break/continue, but we forgot to add it to throw.

[ahe, 2013-04-10 15:17:54]

DBC

https://codereview.chromium.org/14071003/diff/1/tests/language/execute_finall...
File tests/language/execute_finally10_test.dart (right):

https://codereview.chromium.org/14071003/diff/1/tests/language/execute_finall...
tests/language/execute_finally10_test.dart:37: b.run();
Add throw statement here:

throw 'Expected an exception';

https://codereview.chromium.org/14071003/diff/1/tests/language/execute_finall...
tests/language/execute_finally10_test.dart:38: } catch (exception) {
Add // Expected exception.

[ngeoffray, 2013-04-10 15:51:16]

Thanks Peter. I have also improved the CL by fixing nested try/catch, and making
sure a HThrow always has the exit block as a successor. This makes it cleaner by
having a graph with only one leaf.

https://codereview.chromium.org/14071003/diff/1/tests/language/execute_finall...
File tests/language/execute_finally10_test.dart (right):

https://codereview.chromium.org/14071003/diff/1/tests/language/execute_finall...
tests/language/execute_finally10_test.dart:37: b.run();
On 2013/04/10 15:17:54, ahe wrote:
> Add throw statement here:
> 
> throw 'Expected an exception';

Done.

https://codereview.chromium.org/14071003/diff/1/tests/language/execute_finall...
tests/language/execute_finally10_test.dart:38: } catch (exception) {
On 2013/04/10 15:17:54, ahe wrote:
> Add // Expected exception.

Done.

[ngeoffray, 2013-04-11 08:34:59]

I found some problems with this CL, I'll rework it and let you know when it's
ready to review.

[ngeoffray, 2013-04-11 12:18:45]

You can now (re)review the CL. When explaining to Peter what is going on with
the HExitTry, I realized we were doing something wrong when skipping nested
try/catch, so I added a unit test for it.

[kasperl, 2013-04-11 12:28:53]

LGTM.

https://codereview.chromium.org/14071003/diff/11001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/14071003/diff/11001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:3903: close(new
HThrow(exception, isRethrow: true)).addSuccessor(graph.exit);
I'd prefer a new version of close that adds the exit block as a successor.

https://codereview.chromium.org/14071003/diff/11001/tests/language/execute_fi...
File tests/language/execute_finally10_test.dart (right):

https://codereview.chromium.org/14071003/diff/11001/tests/language/execute_fi...
tests/language/execute_finally10_test.dart:25: runCompiler.toString();
Is it impossible to simplify this?

[ngeoffray, 2013-04-11 12:52:28]

Thanks Kasper.

https://codereview.chromium.org/14071003/diff/11001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/14071003/diff/11001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:3903: close(new
HThrow(exception, isRethrow: true)).addSuccessor(graph.exit);
On 2013/04/11 12:28:53, kasperl wrote:
> I'd prefer a new version of close that adds the exit block as a successor.

Done.

https://codereview.chromium.org/14071003/diff/11001/tests/language/execute_fi...
File tests/language/execute_finally10_test.dart (right):

https://codereview.chromium.org/14071003/diff/11001/tests/language/execute_fi...
tests/language/execute_finally10_test.dart:25: runCompiler.toString();
On 2013/04/11 12:28:53, kasperl wrote:
> Is it impossible to simplify this?

I have to use a field twice, to ensure there is a local variable for it. I added
a comment.

[ngeoffray, 2013-04-11 13:20:23]

Committed patchset #5 manually as r21278 (presubmit successful).