Type check try statements

Type check try statements

BUG=
R=karlklose@google.com

Committed: https://code.google.com/p/dart/source/detail?r=23220

[karlklose, 2013-05-27 08:45:22]

LGTM.

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:1114:
!(element.isLibrary() || element.isCompilationUnit())) {
I think
        !element.isLibrary() && !element.isCompilationUnit()
would be easier to read.

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:2983:
exceptionDefinition = node.formals.nodes.head;
Store node.formals.nodes in a local.

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:2986: if
(!node.formals.nodes.tail.tail.isEmpty) {
Also for node.formals.nodes.tail.tail.

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:3040:
compiler.stackTraceClass.computeType(compiler);
Use rawType instead of computeType. I'd like to get rid of computeType soon.

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/typechecker.dart (right):

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/typechecker.dart:809: if
(!indexSetType.parameterTypes.isEmpty) {
You don't need these tests, do you? first/secondType will return the dynamic
type which is assignable.

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/typechecker.dart:1142: //
TODO(johnniwinther): Handle reachability.
Please explain in more detail what is missing.

[Johnni Winther, 2013-05-27 11:01:23]

PTAL

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:1114:
!(element.isLibrary() || element.isCompilationUnit())) {
On 2013/05/27 08:45:22, karlklose wrote:
> I think
>         !element.isLibrary() && !element.isCompilationUnit()
> would be easier to read.

Done.

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:2983:
exceptionDefinition = node.formals.nodes.head;
On 2013/05/27 08:45:22, karlklose wrote:
> Store node.formals.nodes in a local.

Done.

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:2986: if
(!node.formals.nodes.tail.tail.isEmpty) {
On 2013/05/27 08:45:22, karlklose wrote:
> Also for node.formals.nodes.tail.tail.

Done.

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:3040:
compiler.stackTraceClass.computeType(compiler);
On 2013/05/27 08:45:22, karlklose wrote:
> Use rawType instead of computeType. I'd like to get rid of computeType soon.

Done.

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/typechecker.dart (right):

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/typechecker.dart:809: if
(!indexSetType.parameterTypes.isEmpty) {
On 2013/05/27 08:45:22, karlklose wrote:
> You don't need these tests, do you? first/secondType will return the dynamic
> type which is assignable.

I don't test first/secondType. They are used to avoid errors in face of
operators with too few arguments.

[Johnni Winther, 2013-05-27 12:41:07]

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/typechecker.dart (right):

https://codereview.chromium.org/15689009/diff/3013/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/typechecker.dart:1142: //
TODO(johnniwinther): Handle reachability.
On 2013/05/27 08:45:22, karlklose wrote:
> Please explain in more detail what is missing.

Done.

[karlklose, 2013-05-27 12:43:54]

SLGTM.

https://codereview.chromium.org/15689009/diff/14001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/15689009/diff/14001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:2981:
Link<Node> link = node.formals.nodes;
link -> formalsToProcess? formalsLink?

https://codereview.chromium.org/15689009/diff/20001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/15689009/diff/20001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:2982: if
(formalToProcess.isEmpty) {
I think the plural reads better with property accesses.

[Johnni Winther, 2013-05-27 12:48:08]

Committed patchset #7 manually as r23220 (presubmit successful).