Handle malformed types in catch-on.

Handle malformed types in catch-on.


BUG=http://dartbug.com/8325

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

[ngeoffray, 2013-02-05 09:43:46]

LGTM!

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

https://codereview.chromium.org/12220011/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4454:
compiler.cancel('On with no type',
Fits in one line?

https://codereview.chromium.org/12220011/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4459:
graph.addConstantBool(false, constantSystem);
Fits in one line?

[ahe, 2013-02-05 09:58:39]

LGTM!

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

https://codereview.chromium.org/12220011/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4460:
stack.add(condition);
Am I understanding this correctly: the specification say that using a malformed
type in a type test always returns false. It doesn't lead to an exception.

However, in checked mode, an exception often occur, but that is the implicit
type assertion as part of assignment, return, and parameter type checks.

[karlklose, 2013-02-05 10:21:45]

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

https://codereview.chromium.org/12220011/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4460:
stack.add(condition);
Shouldn't we treat any malformed type as dynamic and returning true here?

In checked mode, I think this counts as a subtype test and we have to create a
dynamic type error.

13.2: "In checked mode, it is a dynamic type error if a malformed type is used
in a subtype test. In production mode, an undeclared type is treated as an
instance of type dynamic."

[Johnni Winther, 2013-02-18 14:51:43]

PTAL

The new implementation is not currently exercised since it needs
https://codereview.chromium.org/12210010/.

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

https://codereview.chromium.org/12220011/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4454:
compiler.cancel('On with no type',
On 2013/02/05 09:43:46, ngeoffray wrote:
> Fits in one line?

Done.

https://codereview.chromium.org/12220011/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4459:
graph.addConstantBool(false, constantSystem);
On 2013/02/05 09:43:46, ngeoffray wrote:
> Fits in one line?

:( No.

[ngeoffray, 2013-02-18 15:48:35]

LGTM

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

https://codereview.chromium.org/12220011/diff/6002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4477:
compiler.cancel('On with no type', node: catchBlock.type);
cancel -> internalError ?

https://codereview.chromium.org/12220011/diff/6002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4490: else {
Move else one line above.

https://codereview.chromium.org/12220011/diff/6002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4517: // In checked
mode: Throw a type error if the on-catch type is
Throw -> throw

https://codereview.chromium.org/12220011/diff/6002/tests/language/language.st...
File tests/language/language.status (right):

https://codereview.chromium.org/12220011/diff/6002/tests/language/language.st...
tests/language/language.status:354: on_catch_malformed_type_test: Fail # Issue
8356
Replace with # VM issue ?

https://codereview.chromium.org/12220011/diff/6002/tests/language/language_da...
File tests/language/language_dart2js.status (right):

https://codereview.chromium.org/12220011/diff/6002/tests/language/language_da...
tests/language/language_dart2js.status:202: on_catch_malformed_type_test: Fail #
Malformed types causes compile-time errors.
causes -> cause

https://codereview.chromium.org/12220011/diff/6002/tests/language/on_catch_ma...
File tests/language/on_catch_malformed_type_test.dart (right):

https://codereview.chromium.org/12220011/diff/6002/tests/language/on_catch_ma...
tests/language/on_catch_malformed_type_test.dart:5: // Check that malformed
types in on-catch are handled correct, that is
correct -> correctly

https://codereview.chromium.org/12220011/diff/6002/tests/language/on_catch_ma...
tests/language/on_catch_malformed_type_test.dart:11: String s = i;
How about String s = 1 ?

[Johnni Winther, 2013-02-19 07:08:53]

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

https://codereview.chromium.org/12220011/diff/6002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4477:
compiler.cancel('On with no type', node: catchBlock.type);
On 2013/02/18 15:48:35, ngeoffray wrote:
> cancel -> internalError ?

Done.

https://codereview.chromium.org/12220011/diff/6002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4490: else {
On 2013/02/18 15:48:35, ngeoffray wrote:
> Move else one line above.

Done.

https://codereview.chromium.org/12220011/diff/6002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4517: // In checked
mode: Throw a type error if the on-catch type is
On 2013/02/18 15:48:35, ngeoffray wrote:
> Throw -> throw

Done.

https://codereview.chromium.org/12220011/diff/6002/tests/language/language.st...
File tests/language/language.status (right):

https://codereview.chromium.org/12220011/diff/6002/tests/language/language.st...
tests/language/language.status:354: on_catch_malformed_type_test: Fail # Issue
8356
On 2013/02/18 15:48:35, ngeoffray wrote:
> Replace with # VM issue ?

Done.

https://codereview.chromium.org/12220011/diff/6002/tests/language/language_da...
File tests/language/language_dart2js.status (right):

https://codereview.chromium.org/12220011/diff/6002/tests/language/language_da...
tests/language/language_dart2js.status:202: on_catch_malformed_type_test: Fail #
Malformed types causes compile-time errors.
On 2013/02/18 15:48:35, ngeoffray wrote:
> causes -> cause

Done.

https://codereview.chromium.org/12220011/diff/6002/tests/language/on_catch_ma...
File tests/language/on_catch_malformed_type_test.dart (right):

https://codereview.chromium.org/12220011/diff/6002/tests/language/on_catch_ma...
tests/language/on_catch_malformed_type_test.dart:5: // Check that malformed
types in on-catch are handled correct, that is
On 2013/02/18 15:48:35, ngeoffray wrote:
> correct -> correctly

Done.

https://codereview.chromium.org/12220011/diff/6002/tests/language/on_catch_ma...
tests/language/on_catch_malformed_type_test.dart:11: String s = i;
On 2013/02/18 15:48:35, ngeoffray wrote:
> How about String s = 1 ?

Done.