Track argument types, and remove obsolete code that also used to track it.

Track argument types, and remove obsolete code that also used to track it.
Committed: https://code.google.com/p/dart/source/detail?r=20156

[ngeoffray, 2013-03-18 07:49:55]

Note that this CL requires CL https://codereview.chromium.org/12811010/ to avoid
some performance regressions: now that we infer more in the type inferrer, the
backend inferrer does less, and we need setter constraints to compensate.

[kasperl, 2013-03-18 10:34:51]

LGTM.

https://codereview.chromium.org/12781005/diff/16001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/types/simple_types_inferrer.dart
(right):

https://codereview.chromium.org/12781005/diff/16001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/types/simple_types_inferrer.dart:709:
* Computes the paramter types of [element], based on all call sites we
paramter -> parameter

https://codereview.chromium.org/12781005/diff/16001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/types/simple_types_inferrer.dart:717:
Map<Element, TypeMask> types = new Map<Element, TypeMask>();
Seems a little bit wasteful to have create a new map for every single function
that we analyse, but maybe that isn't so bad.

https://codereview.chromium.org/12781005/diff/16001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/types/simple_types_inferrer.dart:774:
// TODO(ngeoffray): Enable unregistering by moving the logic of
Not sure I fully understand this. Extend comment.

https://codereview.chromium.org/12781005/diff/16001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/types/simple_types_inferrer.dart:917:
: this.named = named == null ? new Map<SourceString, TypeMask>() : named;
I'd suggest () around the == expression.

https://codereview.chromium.org/12781005/diff/16001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/types/simple_types_inferrer.dart:1128:
inferrer.defaultTypeOfParameter[element] = send == null
() around == expression

https://codereview.chromium.org/12781005/diff/16001/tests/compiler/dart2js/ca...
File tests/compiler/dart2js/call_site_simple_type_inferer_test.dart (right):

https://codereview.chromium.org/12781005/diff/16001/tests/compiler/dart2js/ca...
tests/compiler/dart2js/call_site_simple_type_inferer_test.dart:193: x(p1, p2) {}
Is this an intentional edit? Seeing that you expect to give up on TEST_18 anyway
then maybe it would work without the modification?

https://codereview.chromium.org/12781005/diff/16001/tests/compiler/dart2js/ca...
tests/compiler/dart2js/call_site_simple_type_inferer_test.dart:229:
runTest(TEST_3, (inferrer) => [inferrer.numType]);
TEST_3, TEST_4, TEST_8, TEST_9, TEST_10 have worse results here than with the
old optimistic approach. Can we fix that somehow? File a bug?

[ngeoffray, 2013-03-18 11:50:14]

Thanks Kasper.

https://codereview.chromium.org/12781005/diff/16001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/types/simple_types_inferrer.dart
(right):

https://codereview.chromium.org/12781005/diff/16001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/types/simple_types_inferrer.dart:709:
* Computes the paramter types of [element], based on all call sites we
On 2013/03/18 10:34:51, kasperl wrote:
> paramter -> parameter

Done.

https://codereview.chromium.org/12781005/diff/16001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/types/simple_types_inferrer.dart:717:
Map<Element, TypeMask> types = new Map<Element, TypeMask>();
On 2013/03/18 10:34:51, kasperl wrote:
> Seems a little bit wasteful to have create a new map for every single function
> that we analyse, but maybe that isn't so bad.

I removed the creation and changed how we iterate over parameters.

Before it was:
forEachCall
  forEachRequiredParameter
  forEachOptionalParameter
forEachParameter
  update

Now:
forEachParameter
  forEachCall
  update

https://codereview.chromium.org/12781005/diff/16001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/types/simple_types_inferrer.dart:774:
// TODO(ngeoffray): Enable unregistering by moving the logic of
On 2013/03/18 10:34:51, kasperl wrote:
> Not sure I fully understand this. Extend comment.

Done.

https://codereview.chromium.org/12781005/diff/16001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/types/simple_types_inferrer.dart:917:
: this.named = named == null ? new Map<SourceString, TypeMask>() : named;
On 2013/03/18 10:34:51, kasperl wrote:
> I'd suggest () around the == expression.

Done.

https://codereview.chromium.org/12781005/diff/16001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/types/simple_types_inferrer.dart:1128:
inferrer.defaultTypeOfParameter[element] = send == null
On 2013/03/18 10:34:51, kasperl wrote:
> () around == expression

Done.

https://codereview.chromium.org/12781005/diff/16001/tests/compiler/dart2js/ca...
File tests/compiler/dart2js/call_site_simple_type_inferer_test.dart (right):

https://codereview.chromium.org/12781005/diff/16001/tests/compiler/dart2js/ca...
tests/compiler/dart2js/call_site_simple_type_inferer_test.dart:193: x(p1, p2) {}
On 2013/03/18 10:34:51, kasperl wrote:
> Is this an intentional edit? Seeing that you expect to give up on TEST_18
anyway
> then maybe it would work without the modification?

This may have been unintentional. Moved it back to how it was.

https://codereview.chromium.org/12781005/diff/16001/tests/compiler/dart2js/ca...
tests/compiler/dart2js/call_site_simple_type_inferer_test.dart:229:
runTest(TEST_3, (inferrer) => [inferrer.numType]);
On 2013/03/18 10:34:51, kasperl wrote:
> TEST_3, TEST_4, TEST_8, TEST_9, TEST_10 have worse results here than with the
> old optimistic approach. Can we fix that somehow? File a bug?

As discussed, TEST_3 and TEST_4 go from int to num because we're not optimistic
and see (a = 42) and (a = a - 42), which infer to num. I'm not too worried about
moving from int to num in those cases.

For TEST_8 TEST_9 and TEST_10, we're dealing with recursive calls that use the
parameters we're trying to infer. We need a constraint system to solve this. I
added a comment.

[ngeoffray, 2013-03-18 13:05:29]

I had to mark as fail two tests in checked mode, because we now know the type of
a parameter is a list, and the backend manually inlines the code array[1.0] to
do a ArgumentError. In checked mode, it should be a TypeError.

I filed bug http://code.google.com/p/dart/issues/detail?id=9228.

[ngeoffray, 2013-03-18 13:06:01]

Committed patchset #3 manually as r20156 (presubmit successful).