Include ClosureInfo in dump info.

Include ClosureInfo in dump info.

Uses the 0.5.0 version of dart2js_info

R=sigmund@google.com

Committed: https://github.com/dart-lang/sdk/commit/8d746645e38fcdcdde2767b2dbf03d7d3e806486

[Harry Terkelsen, 2016-10-06 17:38:20]

het@google.com changed reviewers:
+ sigmund@google.com

[Siggi Cherem (dart-lang), 2016-10-06 17:58:09]

lgtm

https://codereview.chromium.org/2398133002/diff/1/pkg/compiler/lib/src/dump_i...
File pkg/compiler/lib/src/dump_info.dart (right):

https://codereview.chromium.org/2398133002/diff/1/pkg/compiler/lib/src/dump_i...
pkg/compiler/lib/src/dump_info.dart:147: List<ClosureInfo> nestedClosures =
<ClosureInfo>[];
the code here and in line 313 seem almost the same, let's move it into it's own
method.

https://codereview.chromium.org/2398133002/diff/1/pkg/compiler/lib/src/dump_i...
pkg/compiler/lib/src/dump_info.dart:187: for (Element function in
member.nestedClosures) {
we might be able to use the same function here I think (we just don't use the
computed list of `nestedClosures` infos)

https://codereview.chromium.org/2398133002/diff/1/pkg/compiler/lib/src/dump_i...
pkg/compiler/lib/src/dump_info.dart:223: if (functionInfo == null) return null;
when can this be null?

https://codereview.chromium.org/2398133002/diff/1/pkg/compiler/lib/src/dump_i...
pkg/compiler/lib/src/dump_info.dart:316: ClosureInfo closure =
this.process(callMethod.enclosingClass);
is .enclosingClass the same as .enclosingElement (and also the same as
.closureClass)?

If so - maybe I'd use `.closureClass`, it's a bit more self-documenting.

https://codereview.chromium.org/2398133002/diff/1/pkg/compiler/pubspec.yaml
File pkg/compiler/pubspec.yaml (left):

https://codereview.chromium.org/2398133002/diff/1/pkg/compiler/pubspec.yaml#o...
pkg/compiler/pubspec.yaml:1: # This pubspec is currently mainly used to make it
easier to develop on dart2js
I guess now that we have the .packages at the top of the repo, we don't need
this locally, but I wonder if it's worth keeping it for any other reason.

In the case of js_ast, js_runtime, etc, I think it helps make it easier to push
out "compiler_unsupported" (used in dartpad), so maybe this is still worth
keeping too?

[Harry Terkelsen, 2016-10-06 18:22:43]

Thanks, Siggi!

https://codereview.chromium.org/2398133002/diff/1/pkg/compiler/lib/src/dump_i...
File pkg/compiler/lib/src/dump_info.dart (right):

https://codereview.chromium.org/2398133002/diff/1/pkg/compiler/lib/src/dump_i...
pkg/compiler/lib/src/dump_info.dart:147: List<ClosureInfo> nestedClosures =
<ClosureInfo>[];
On 2016/10/06 17:58:09, Siggi Cherem (dart-lang) wrote:
> the code here and in line 313 seem almost the same, let's move it into it's
own
> method.

Done.

https://codereview.chromium.org/2398133002/diff/1/pkg/compiler/lib/src/dump_i...
pkg/compiler/lib/src/dump_info.dart:187: for (Element function in
member.nestedClosures) {
On 2016/10/06 17:58:09, Siggi Cherem (dart-lang) wrote:
> we might be able to use the same function here I think (we just don't use the
> computed list of `nestedClosures` infos)

I think we can just iterate over all closures in the info and add the size
instead of recomputing the info unnecessarily.

https://codereview.chromium.org/2398133002/diff/1/pkg/compiler/lib/src/dump_i...
pkg/compiler/lib/src/dump_info.dart:223: if (functionInfo == null) return null;
On 2016/10/06 17:58:08, Siggi Cherem (dart-lang) wrote:
> when can this be null?

If the closure is not actually output in the code or inlined.

https://codereview.chromium.org/2398133002/diff/1/pkg/compiler/lib/src/dump_i...
pkg/compiler/lib/src/dump_info.dart:316: ClosureInfo closure =
this.process(callMethod.enclosingClass);
On 2016/10/06 17:58:09, Siggi Cherem (dart-lang) wrote:
> is .enclosingClass the same as .enclosingElement (and also the same as
> .closureClass)?
> 
> If so - maybe I'd use `.closureClass`, it's a bit more self-documenting.

Done.

https://codereview.chromium.org/2398133002/diff/1/pkg/compiler/pubspec.yaml
File pkg/compiler/pubspec.yaml (left):

https://codereview.chromium.org/2398133002/diff/1/pkg/compiler/pubspec.yaml#o...
pkg/compiler/pubspec.yaml:1: # This pubspec is currently mainly used to make it
easier to develop on dart2js
On 2016/10/06 17:58:08, Siggi Cherem (dart-lang) wrote:
> I guess now that we have the .packages at the top of the repo, we don't need
> this locally, but I wonder if it's worth keeping it for any other reason.
> 
> In the case of js_ast, js_runtime, etc, I think it helps make it easier to
push
> out "compiler_unsupported" (used in dartpad), so maybe this is still worth
> keeping too?

Done.

[Harry Terkelsen, 2016-10-06 20:48:56]

Description was changed from

==========
Include ClosureInfo in dump info.

Uses the 0.5.0 version of dart2js_info
==========

to

==========
Include ClosureInfo in dump info.

Uses the 0.5.0 version of dart2js_info

R=sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/8d746645e38fcdcdde2767b2dbf03d7d3e806486
==========

[Harry Terkelsen, 2016-10-06 20:48:57]

Committed patchset #2 (id:20001) manually as
8d746645e38fcdcdde2767b2dbf03d7d3e806486 (presubmit successful).