Follow factory redirections when checking case expressions.

Follow factory redirections when checking case expressions.

This version does the check in the resolver and uses the constant values and not just the types. As such, it also follows the constructor chain.

R=karlklose@google.com

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

[floitsch, 2014-02-28 22:00:27]

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

https://codereview.chromium.org/184663004/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:3606:
addDeferredAction(enclosingElement, () {
I need the constants to be evaluated. I'm not sure if this is the right way to
do this, though.

https://codereview.chromium.org/184663004/diff/20001/tests/co19/co19-dart2js....
File tests/co19/co19-dart2js.status (right):

https://codereview.chromium.org/184663004/diff/20001/tests/co19/co19-dart2js....
tests/co19/co19-dart2js.status:142: Language/13_Statements/09_Switch_A03_t03:
MissingCompileTimeError # Issue TODO(floitsch): reuse issue number.
@johnni: the reason this now fails is because there is a switch with a double
that has an integer value:

switch (o) { case 3.0: ... }

Our constant system says, this is an int, and therefore doesn't complain
anymore.

This is something you have been working on. Do you have an issue number?

[floitsch, 2014-03-03 09:57:43]

Had to update the type-checker test. The same-type checks are now done in the
resolver, and the type-checker test therefore doesn't need to verify the
switch-cases.

[Johnni Winther, 2014-03-03 10:01:57]

On 2014/02/28 22:00:27, floitsch wrote:
>
https://codereview.chromium.org/184663004/diff/20001/sdk/lib/_internal/compil...
> File sdk/lib/_internal/compiler/implementation/resolution/members.dart
(right):
> 
>
https://codereview.chromium.org/184663004/diff/20001/sdk/lib/_internal/compil...
> sdk/lib/_internal/compiler/implementation/resolution/members.dart:3606:
> addDeferredAction(enclosingElement, () {
> I need the constants to be evaluated. I'm not sure if this is the right way to
> do this, though.
> 
>
https://codereview.chromium.org/184663004/diff/20001/tests/co19/co19-dart2js....
> File tests/co19/co19-dart2js.status (right):
> 
>
https://codereview.chromium.org/184663004/diff/20001/tests/co19/co19-dart2js....
> tests/co19/co19-dart2js.status:142: Language/13_Statements/09_Switch_A03_t03:
> MissingCompileTimeError # Issue TODO(floitsch): reuse issue number.
> @johnni: the reason this now fails is because there is a switch with a double
> that has an integer value:
> 
> switch (o) { case 3.0: ... }
> 
> Our constant system says, this is an int, and therefore doesn't complain
> anymore.
> 
> This is something you have been working on. Do you have an issue number?

I originally intended to include the fix in
https://codereview.chromium.org/146733003/ but the issue is to huge and needs
its own CL.

[floitsch, 2014-03-03 10:36:19]

Updated issue number.

[karlklose, 2014-03-03 10:38:05]

LGTM.

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

https://codereview.chromium.org/184663004/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:3469: if
(constant.isInt()) return compiler.intClass.computeType(compiler);
These should use `rawType` instead of `computeType`.

The type-resolver should have computed thisType and rawType for these classes.

https://codereview.chromium.org/184663004/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:3475: return
compiler.functionClass.computeType(compiler);
Ditto.

https://codereview.chromium.org/184663004/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:3521:
compiler.reportError(node, MessageKind.SWITCH_CASE_FORBIDDEN,
Is the problem here that double implements '=='?  We could issue a more helpful
warning then.

https://codereview.chromium.org/184663004/diff/100001/tests/co19/co19-dart2js...
File tests/co19/co19-dart2js.status (right):

https://codereview.chromium.org/184663004/diff/100001/tests/co19/co19-dart2js...
tests/co19/co19-dart2js.status:142: Language/13_Statements/09_Switch_A03_t03:
MissingCompileTimeError # Issue TODO(floitsch): reuse issue number.
What issue number?

https://codereview.chromium.org/184663004/diff/100001/tests/compiler/dart2js/...
File tests/compiler/dart2js/type_checker_test.dart (left):

https://codereview.chromium.org/184663004/diff/100001/tests/compiler/dart2js/...
tests/compiler/dart2js/type_checker_test.dart:155: analyze("switch (null) { case
'': break; case 2: break; }",
You could move this test to the resolver test instead of removing it.

[floitsch, 2014-03-03 18:47:15]

PTAL at the resolver test.

I will commit as is, but can change in a follow-up CL.

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

https://codereview.chromium.org/184663004/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:3469: if
(constant.isInt()) return compiler.intClass.computeType(compiler);
On 2014/03/03 10:38:05, karlklose wrote:
> These should use `rawType` instead of `computeType`.
> 
> The type-resolver should have computed thisType and rawType for these classes.

Done.

https://codereview.chromium.org/184663004/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:3475: return
compiler.functionClass.computeType(compiler);
On 2014/03/03 10:38:05, karlklose wrote:
> Ditto.

Done.

https://codereview.chromium.org/184663004/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:3521:
compiler.reportError(node, MessageKind.SWITCH_CASE_FORBIDDEN,
On 2014/03/03 10:38:05, karlklose wrote:
> Is the problem here that double implements '=='?  We could issue a more
helpful
> warning then.

Reusing OVERRIDES_EQUALS.

https://codereview.chromium.org/184663004/diff/100001/tests/co19/co19-dart2js...
File tests/co19/co19-dart2js.status (right):

https://codereview.chromium.org/184663004/diff/100001/tests/co19/co19-dart2js...
tests/co19/co19-dart2js.status:142: Language/13_Statements/09_Switch_A03_t03:
MissingCompileTimeError # Issue TODO(floitsch): reuse issue number.
On 2014/03/03 10:38:05, karlklose wrote:
> What issue number?

I thought there was already an open one. There wasn't, so I created a new one.

https://codereview.chromium.org/184663004/diff/100001/tests/compiler/dart2js/...
File tests/compiler/dart2js/type_checker_test.dart (left):

https://codereview.chromium.org/184663004/diff/100001/tests/compiler/dart2js/...
tests/compiler/dart2js/type_checker_test.dart:155: analyze("switch (null) { case
'': break; case 2: break; }",
On 2014/03/03 10:38:05, karlklose wrote:
> You could move this test to the resolver test instead of removing it.

Done.

[floitsch, 2014-03-03 19:22:24]

Committed patchset #10 manually as r33239 (presubmit successful).