Attribute closure sizes to their containing functions and closures.

Attribute closure sizes to their containing functions and closures.

BUG=
R=johnniwinther@google.com, sra@google.com

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

[sra1, 2014-08-04 23:25:59]

lgtm.

Does it pick up all the closures in:
----------
main() {
  print(foo);
  print(foo());
  print(new X<String>().foo);
}

foo() => [() => [() => [() => [() => [() => [() => [() => [() => [() => [() =>
[() => []]]]]]]]]]]];

class X {
  foo() => [() => [() => [() => [() => [() => [() => [() => [() => [() => [() =>
[() => []]]]]]]]]]]];
}

----------

https://codereview.chromium.org/435263002/diff/1/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/dump_info.dart (right):

https://codereview.chromium.org/435263002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/dump_info.dart:248: Map<String,
dynamic> child = this.process(closure);
Does this cause the closure to be processed twice?
If so, can that cause O(n^2) behaviour or not?

[Ty Overby (Google), 2014-08-04 23:31:55]

On 2014/08/04 23:25:59, sra1 wrote:
> lgtm.
> 
> Does it pick up all the closures in:
> ----------
> main() {
>   print(foo);
>   print(foo());
>   print(new X<String>().foo);
> }
> 
> foo() => [() => [() => [() => [() => [() => [() => [() => [() => [() => [() =>
> [() => []]]]]]]]]]]];
> 
> class X {
>   foo() => [() => [() => [() => [() => [() => [() => [() => [() => [() => [()
=>
> [() => []]]]]]]]]]]];
> }

I'll use this as a test case and get back to you.

> 
> ----------
> 
>
https://codereview.chromium.org/435263002/diff/1/sdk/lib/_internal/compiler/i...
> File sdk/lib/_internal/compiler/implementation/dump_info.dart (right):
> 
>
https://codereview.chromium.org/435263002/diff/1/sdk/lib/_internal/compiler/i...
> sdk/lib/_internal/compiler/implementation/dump_info.dart:248: Map<String,
> dynamic> child = this.process(closure);
> Does this cause the closure to be processed twice?
> If so, can that cause O(n^2) behaviour or not?

[Ty Overby (Google), 2014-08-04 23:32:02]

https://codereview.chromium.org/435263002/diff/1/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/dump_info.dart (right):

https://codereview.chromium.org/435263002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/dump_info.dart:248: Map<String,
dynamic> child = this.process(closure);
On 2014/08/04 23:25:59, sra1 wrote:
> Does this cause the closure to be processed twice?
> If so, can that cause O(n^2) behaviour or not?

The `process` function is memoized.  It actually exists for exactly this
purpose.

[sra1, 2014-08-05 21:26:28]

still lgtm

[Ty Overby (Google), 2014-08-05 22:39:30]

On 2014/08/05 21:26:28, sra1 wrote:
> still lgtm

Could you take a look at the current change?  
I added in a few checks.  Also when running my local test suite, 
dartanalyzer complained that MethodElement isn't used.  This patch set 
adds that file to the whitelist.  Should I remove the class instead?

[sra1, 2014-08-05 23:05:37]

1. Create the issue for Johnni
2. Add a TODO(NNN) 
3. Fix the other nit.

then LGTM (with the whitelist entry)

Johnni: The refactoring r38381 seems to have introduced MethodElement with no
implementing instances.  What is the intention here?

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

https://codereview.chromium.org/435263002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/dump_info.dart:324: for (Element
closure in (element as MemberElement).nestedClosures) {
We prefer a declaration with a type assertion:

MemberElement member = element;
for (Element closure in member.nestedClosures) ...

https://codereview.chromium.org/435263002/diff/20001/tests/compiler/dart2js/a...
File tests/compiler/dart2js/analyze_unused_dart2js_test.dart (right):

https://codereview.chromium.org/435263002/diff/20001/tests/compiler/dart2js/a...
tests/compiler/dart2js/analyze_unused_dart2js_test.dart:28: // MethodElement
File an issue assigned to Johnni to investigate why MethodElement is unused.

 // TODO(NNN): Why is MethodElement unused?

where NNN is the issue number.

[Johnni Winther, 2014-08-06 08:50:04]

Whitelisting lgtm.

https://codereview.chromium.org/435263002/diff/40001/tests/compiler/dart2js/a...
File tests/compiler/dart2js/analyze_unused_dart2js_test.dart (right):

https://codereview.chromium.org/435263002/diff/40001/tests/compiler/dart2js/a...
tests/compiler/dart2js/analyze_unused_dart2js_test.dart:29: // TODO(20377): Why
is MethodElement unused?
It is a newly introduced element. Use is in progress.

[Ty Overby (Google), 2014-08-06 17:57:38]

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

[floitsch, 2014-08-11 16:03:17]

DBC.

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

https://codereview.chromium.org/435263002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/dump_info.dart:324: MemberElement
member = element as MemberElement;
no need to write "as ...".

[floitsch, 2014-08-11 16:04:55]

On 2014/08/11 16:03:17, floitsch wrote:
> DBC.
> 
>
https://codereview.chromium.org/435263002/diff/40001/sdk/lib/_internal/compil...
> File sdk/lib/_internal/compiler/implementation/dump_info.dart (right):
> 
>
https://codereview.chromium.org/435263002/diff/40001/sdk/lib/_internal/compil...
> sdk/lib/_internal/compiler/implementation/dump_info.dart:324: MemberElement
> member = element as MemberElement;
> no need to write "as ...".

And it would be nice to have Stephen's example as a test-case.

[Ty Overby (Google), 2014-08-13 22:02:06]

On 2014/08/11 16:04:55, floitsch wrote:
> On 2014/08/11 16:03:17, floitsch wrote:
> > DBC.
> > 
> >
>
https://codereview.chromium.org/435263002/diff/40001/sdk/lib/_internal/compil...
> > File sdk/lib/_internal/compiler/implementation/dump_info.dart (right):
> > 
> >
>
https://codereview.chromium.org/435263002/diff/40001/sdk/lib/_internal/compil...
> > sdk/lib/_internal/compiler/implementation/dump_info.dart:324: MemberElement
> > member = element as MemberElement;
> > no need to write "as ...".
> 
> And it would be nice to have Stephen's example as a test-case.

Stephen's example has been added in https://codereview.chromium.org/469903002/