Implement tracing for function expressions and statements, and infer types of parameters of these c…

Implement tracing for function expressions and statements, and infer types of parameters of these closures.

R=kasperl@google.com

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

[ngeoffray, 2013-12-10 17:11:02]

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/inferrer/container_tracer.dart
(right):

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/container_tracer.dart:1: //
Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
I plan to split this file into different files in another CL.

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/universe/universe.dart (left):

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/universe/universe.dart:324:
assert(sameNameHack(element, compiler));
This is now also being called for closure calls, where the element name is the
closure name, and the selector name can be anything, depending on how the
closure is called.

[kasperl, 2013-12-11 07:35:34]

LGTM.

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/inferrer/container_tracer.dart
(right):

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/container_tracer.dart:308: if
(info.targets
Consider using a local variable for the result of the map. Something like:

   Iterable<...> inferredTargetTypes = info.targets.map((element) =>
       inferrer.types.getInferredTypeOf(element));
   if (inferredTargetTypes.any((user) => currentUser == user)) {
     ...
   }

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/container_tracer.dart:371:
super.visitStaticCallSiteTypeInformation(info);
Is it important to call the super method at the bottom of the method? It seems
"cleaner" to always call it at the beginning (repeated in a number of places).

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart
(right):

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:213: //
of closured that are traced can be inferred.
closured -> closures?

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:214:
bool turnSpecialNonSpecial = false;
disableHandleSpecialCases?

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/universe/universe.dart (left):

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/universe/universe.dart:324:
assert(sameNameHack(element, compiler));
On 2013/12/10 17:11:02, ngeoffray wrote:
> This is now also being called for closure calls, where the element name is the
> closure name, and the selector name can be anything, depending on how the
> closure is called.

Should we get rid of all the sameNameHack stuff?

https://codereview.chromium.org/111803002/diff/40001/tests/compiler/dart2js/c...
File tests/compiler/dart2js/closure_tracer_test.dart (right):

https://codereview.chromium.org/111803002/diff/40001/tests/compiler/dart2js/c...
tests/compiler/dart2js/closure_tracer_test.dart:49: main() {
Add test for "stored" in parameter?

[ngeoffray, 2013-12-16 11:47:43]

Thanks Kasper.

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/inferrer/container_tracer.dart
(right):

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/container_tracer.dart:308: if
(info.targets
On 2013/12/11 07:35:35, kasperl wrote:
> Consider using a local variable for the result of the map. Something like:
> 
>    Iterable<...> inferredTargetTypes = info.targets.map((element) =>
>        inferrer.types.getInferredTypeOf(element));
>    if (inferredTargetTypes.any((user) => currentUser == user)) {
>      ...
>    }

Done.

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/container_tracer.dart:371:
super.visitStaticCallSiteTypeInformation(info);
On 2013/12/11 07:35:35, kasperl wrote:
> Is it important to call the super method at the bottom of the method? It seems
> "cleaner" to always call it at the beginning (repeated in a number of places).

Done.

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart
(right):

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:213: //
of closured that are traced can be inferred.
On 2013/12/11 07:35:35, kasperl wrote:
> closured -> closures?

Done.

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:214:
bool turnSpecialNonSpecial = false;
On 2013/12/11 07:35:35, kasperl wrote:
> disableHandleSpecialCases?

Done.

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/universe/universe.dart (left):

https://codereview.chromium.org/111803002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/universe/universe.dart:324:
assert(sameNameHack(element, compiler));
On 2013/12/11 07:35:35, kasperl wrote:
> On 2013/12/10 17:11:02, ngeoffray wrote:
> > This is now also being called for closure calls, where the element name is
the
> > closure name, and the selector name can be anything, depending on how the
> > closure is called.
> 
> Should we get rid of all the sameNameHack stuff?

I'll try, but some call sites do tricky things hard to work around.

https://codereview.chromium.org/111803002/diff/40001/tests/compiler/dart2js/c...
File tests/compiler/dart2js/closure_tracer_test.dart (right):

https://codereview.chromium.org/111803002/diff/40001/tests/compiler/dart2js/c...
tests/compiler/dart2js/closure_tracer_test.dart:49: main() {
On 2013/12/11 07:35:35, kasperl wrote:
> Add test for "stored" in parameter?

Done.

[ngeoffray, 2013-12-16 15:38:38]

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