Add fasta-based summary generation code.

Add fasta-based summary generation code.

The code is still a work in progress, so its test is marked as failing.

Note: I believe there is code duplicatoin between the StackListener
class introduced by this CL and the StackListener class in
pkg/front_end.  Once more tests are passing, I intend to remove this
code duplication.

R=sigmund@google.com

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

[Paul Berry, 2017-03-09 19:18:07]

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

[Siggi Cherem (dart-lang), 2017-03-09 21:02:14]

lgtm, thanks Paul

mostly questions really :)

https://codereview.chromium.org/2738273002/diff/20001/pkg/analyzer/lib/src/su...
File pkg/analyzer/lib/src/summary/fasta/summary_builder.dart (right):

https://codereview.chromium.org/2738273002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/fasta/summary_builder.dart:10: import
'package:front_end/src/fasta/parser/class_member_parser.dart';
I'm surprised that the sorting of imports doesn't respect the split between
local and package-imports.

I personally prefer to have that split, but we can keep it if that's hard to
change.

https://codereview.chromium.org/2738273002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/fasta/summary_builder.dart:406: // TODO(paulberry):
Copied from beginFieldInitializer.  Is all of this needed?
did you mean from beginInitializer?

https://codereview.chromium.org/2738273002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/fasta/summary_builder.dart:425: // TODO(paulberry):
copied from endInitializer.  Is all of this needed?
just to make sure I follow - you are wondering if we can simplify how we handle
the initializer expressions or something else?

https://codereview.chromium.org/2738273002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/fasta/summary_builder.dart:448: throw new
UnimplementedError(); // TODO(paulberry): does this ever occur?
do you mean that we don't summarize enums altogether?

https://codereview.chromium.org/2738273002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/fasta/summary_builder.dart:1401: //
TODO(paulberry): is this needed?
really depends on how we want to handle recovery of invalid constant
expressions. If we report the error elsewhere and we don't expect to run the
summary builder on invalid code, then we don't need it.

https://codereview.chromium.org/2738273002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/fasta/summary_builder.dart:1440: throw new
UnimplementedError(); // TODO(paulberry): fix the code below.
is the fix related to references in general?

[Paul Berry, 2017-03-09 23:26:23]

https://codereview.chromium.org/2738273002/diff/20001/pkg/analyzer/lib/src/su...
File pkg/analyzer/lib/src/summary/fasta/summary_builder.dart (right):

https://codereview.chromium.org/2738273002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/fasta/summary_builder.dart:10: import
'package:front_end/src/fasta/parser/class_member_parser.dart';
On 2017/03/09 21:02:14, Siggi Cherem (dart-lang) wrote:
> I'm surprised that the sorting of imports doesn't respect the split between
> local and package-imports.
> 
> I personally prefer to have that split, but we can keep it if that's hard to
> change.

Oops--I added these imports in any old location because I was intending to use
analysis server's "sort" function on the file later and fix it up.  I forgot to
sort them afterwards.

I'll sort before landing the CL.

https://codereview.chromium.org/2738273002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/fasta/summary_builder.dart:406: // TODO(paulberry):
Copied from beginFieldInitializer.  Is all of this needed?
On 2017/03/09 21:02:13, Siggi Cherem (dart-lang) wrote:
> did you mean from beginInitializer?

Oops, yes.  I've removed this TODO--it was a placeholder I put in before I
understood the difference between beginInitializer and beginFieldInitializer.

https://codereview.chromium.org/2738273002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/fasta/summary_builder.dart:425: // TODO(paulberry):
copied from endInitializer.  Is all of this needed?
On 2017/03/09 21:02:14, Siggi Cherem (dart-lang) wrote:
> just to make sure I follow - you are wondering if we can simplify how we
handle
> the initializer expressions or something else?

At the time I wrote the TODO I didn't understand the difference between
endInitializer and endFieldInitializer, so I wasn't sure which function the code
belonged in.  Later when I figured out that it *does* belong here, I forgot to
remove the TODO.

I've removed it now.

https://codereview.chromium.org/2738273002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/fasta/summary_builder.dart:448: throw new
UnimplementedError(); // TODO(paulberry): does this ever occur?
On 2017/03/09 21:02:14, Siggi Cherem (dart-lang) wrote:
> do you mean that we don't summarize enums altogether?

We do summarize enums, but the parsing of enums never calls "endFields".  So I
believe thes code is unreachable.

https://codereview.chromium.org/2738273002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/fasta/summary_builder.dart:1401: //
TODO(paulberry): is this needed?
On 2017/03/09 21:02:14, Siggi Cherem (dart-lang) wrote:
> really depends on how we want to handle recovery of invalid constant
> expressions. If we report the error elsewhere and we don't expect to run the
> summary builder on invalid code, then we don't need it.

We *do* expect to run the summary builder on invalid code.  The only reason this
is commented out for now is because there are currently no callers, so analyzer
was giving me an "unused method" hint.  I'll remove the TODO comment when I fix
this.

https://codereview.chromium.org/2738273002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/fasta/summary_builder.dart:1440: throw new
UnimplementedError(); // TODO(paulberry): fix the code below.
On 2017/03/09 21:02:13, Siggi Cherem (dart-lang) wrote:
> is the fix related to references in general?

I don't know what the proper fix is.  The reason I commented it out is because
the line below produces an analyzer error: TypeRef requires two arguments, not
one.

I plan to fix this code as part of getting the rest of the summary tests to
pass.

[Siggi Cherem (dart-lang), 2017-03-09 23:35:50]

thanks for all the clarifications! It all makes sense.

[Paul Berry, 2017-03-09 23:38:46]

Description was changed from

==========
Add fasta-based summary generation code.

The code is still a work in progress, so its test is marked as failing.

Note: I believe there is code duplicatoin between the StackListener
class introduced by this CL and the StackListener class in
pkg/front_end.  Once more tests are passing, I intend to remove this
code duplication.
==========

to

==========
Add fasta-based summary generation code.

The code is still a work in progress, so its test is marked as failing.

Note: I believe there is code duplicatoin between the StackListener
class introduced by this CL and the StackListener class in
pkg/front_end.  Once more tests are passing, I intend to remove this
code duplication.

R=sigmund@google.com

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

[Paul Berry, 2017-03-09 23:38:47]

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