dart2js: add visitors and parsing support to infos

dart2js: add visitors and parsing support to infos

R=het@google.com, johnniwinther@google.com

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

[Siggi Cherem (dart-lang), 2015-08-11 00:37:15]

Patchset #1 (id:1) has been deleted

[Siggi Cherem (dart-lang), 2015-08-11 00:37:20]

Patchset #1 (id:20001) has been deleted

[Siggi Cherem (dart-lang), 2015-08-11 00:41:46]

sigmund@google.com changed reviewers:
+ het@google.com, johnniwinther@google.com, sra@google.com

[Harry Terkelsen, 2015-08-11 01:15:19]

lgtm

https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
File pkg/compiler/lib/src/info/info.dart (right):

https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/info/info.dart:44: final String kind;
consider making kind an enum?

https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/info/info.dart:346: final List<FunctionInfo>
topLevelFunctions = <FunctionInfo>[];
document all of the fields for the Info classes

https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/info/info.dart:629: visitTypedef(TypedefInfo info) {}
remove or make InfoVisitor abstract

https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/info/info.dart:630: visitOutput(OutputUnitInfo info) {}
ditto

[Johnni Winther, 2015-08-11 08:45:37]

lgtm

https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
File pkg/compiler/lib/src/info/info.dart (right):

https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/info/info.dart:204: class _ParseHelper {
Do we have a test of the serialization/deserialization so we don't unintendedly
miss fields?

[Siggi Cherem (dart-lang), 2015-08-11 16:09:27]

thanks for the review!

https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
File pkg/compiler/lib/src/info/info.dart (right):

https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/info/info.dart:44: final String kind;
On 2015/08/11 01:15:18, Harry Terkelsen wrote:
> consider making kind an enum?

It's interesting because I only had in mind to use kind for serialization. I
didn't initially intend for the kind to be used to compare for the kind of info,
but now that you mention this, I can see how that's a natural thing to do (end
even I used it in dump_info.dart!).

I think we should use the type itself for that purpose (`e is ClassInfo` instead
of `e.kind == InfoKinds.classInfo`). I',m wondering whether we should make this
field private as well?

I added the enum here for now, but enums are so restricted in Dart, that the
code is more complex as a result. I'm not sure if it's worth it. WDYT?

https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/info/info.dart:204: class _ParseHelper {
On 2015/08/11 08:45:37, Johnni Winther wrote:
> Do we have a test of the serialization/deserialization so we don't
unintendedly
> miss fields?

Not yet, but we will. Added a TODO

https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/info/info.dart:346: final List<FunctionInfo>
topLevelFunctions = <FunctionInfo>[];
On 2015/08/11 01:15:18, Harry Terkelsen wrote:
> document all of the fields for the Info classes

Done.

https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/info/info.dart:629: visitTypedef(TypedefInfo info) {}
On 2015/08/11 01:15:18, Harry Terkelsen wrote:
> remove or make InfoVisitor abstract

Done.

https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/info/info.dart:630: visitOutput(OutputUnitInfo info) {}
On 2015/08/11 01:15:18, Harry Terkelsen wrote:
> ditto

Done.

[Siggi Cherem (dart-lang), 2015-08-11 16:11:07]

Committed patchset #2 (id:60001) manually as
fd9b56078826ff893dc8845e0e60a67b96d1761b (presubmit successful).

[Siggi Cherem (dart-lang), 2015-08-11 16:16:43]

https://codereview.chromium.org/1285743002/diff/60001/pkg/compiler/lib/src/du...
File pkg/compiler/lib/src/dump_info.dart (right):

https://codereview.chromium.org/1285743002/diff/60001/pkg/compiler/lib/src/du...
pkg/compiler/lib/src/dump_info.dart:482: Info outerInfo =
infoCollector._elementToInfo[element];
oops - I noticed this error while testing things out in checked mode, and so I
included it with this same CL without realizing it. Turns out this added a
warning to dart2js, so I sent a follow up CL fixing it.

[Harry Terkelsen, 2015-08-11 20:11:55]

https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
File pkg/compiler/lib/src/info/info.dart (right):

https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/info/info.dart:44: final String kind;
On 2015/08/11 16:09:27, Siggi Cherem (dart-lang) wrote:
> On 2015/08/11 01:15:18, Harry Terkelsen wrote:
> > consider making kind an enum?
> 
> It's interesting because I only had in mind to use kind for serialization. I
> didn't initially intend for the kind to be used to compare for the kind of
info,
> but now that you mention this, I can see how that's a natural thing to do (end
> even I used it in dump_info.dart!).
> 
> I think we should use the type itself for that purpose (`e is ClassInfo`
instead
> of `e.kind == InfoKinds.classInfo`). I',m wondering whether we should make
this
> field private as well?
> 
> I added the enum here for now, but enums are so restricted in Dart, that the
> code is more complex as a result. I'm not sure if it's worth it. WDYT?

I've run into the same problem when trying to enum-ify Dart code. If it makes
things more complex then I think it's better to just leave it.

[Siggi Cherem (dart-lang), 2015-08-11 20:13:40]

On 2015/08/11 20:11:55, Harry Terkelsen wrote:
>
https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
> File pkg/compiler/lib/src/info/info.dart (right):
> 
>
https://codereview.chromium.org/1285743002/diff/40001/pkg/compiler/lib/src/in...
> pkg/compiler/lib/src/info/info.dart:44: final String kind;
> On 2015/08/11 16:09:27, Siggi Cherem (dart-lang) wrote:
> > On 2015/08/11 01:15:18, Harry Terkelsen wrote:
> > > consider making kind an enum?
> > 
> > It's interesting because I only had in mind to use kind for serialization. I
> > didn't initially intend for the kind to be used to compare for the kind of
> info,
> > but now that you mention this, I can see how that's a natural thing to do
(end
> > even I used it in dump_info.dart!).
> > 
> > I think we should use the type itself for that purpose (`e is ClassInfo`
> instead
> > of `e.kind == InfoKinds.classInfo`). I',m wondering whether we should make
> this
> > field private as well?
> > 
> > I added the enum here for now, but enums are so restricted in Dart, that the
> > code is more complex as a result. I'm not sure if it's worth it. WDYT?
> 
> I've run into the same problem when trying to enum-ify Dart code. If it makes
> things more complex then I think it's better to just leave it.

Thanks - I did submit the change together with this CL, but I might revert it
later :)