fasta.CommentToken implement analyzer.CommentToken

fasta.CommentToken implement analyzer.CommentToken

* update the fasta scanner to generate comment and dartdoc tokens

* replace fasta.preceedingComments field
  with preceedingCommentTokens field and preceedingComments getter/setter

* update token translator

R=paulberry@google.com

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

[danrubel, 2017-03-21 01:12:49]

danrubel@google.com changed reviewers:
+ ahe@google.com, paulberry@google.com

[Paul Berry, 2017-03-21 14:23:07]

lgtm

https://codereview.chromium.org/2763833002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/scanner/token.dart (right):

https://codereview.chromium.org/2763833002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/scanner/token.dart:457: throw 'unsupported
operation';
Change this to `throw new StateError('unsupported operation');`

Rationale: Error objects contain a stackTrace member which is automatically set
by the VM at the time the error is first thrown.  If it is later caught and
re-thrown, it is not changed.  This makes it easier to track down the source of
errors when they occur.

https://codereview.chromium.org/2763833002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/scanner/token.dart:462: // TODO: implement remove
Nit: change this to TODO(danrubel)

https://codereview.chromium.org/2763833002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/scanner/token.dart:463: throw 'not implemented yet';
Similarly, change to `throw new UnimplementedError();`

[ahe, 2017-03-21 14:36:50]

https://codereview.chromium.org/2763833002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/scanner/token.dart (right):

https://codereview.chromium.org/2763833002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/scanner/token.dart:457: throw 'unsupported
operation';
On 2017/03/21 14:23:07, Paul Berry wrote:
> Change this to `throw new StateError('unsupported operation');`
> 
> Rationale: Error objects contain a stackTrace member which is automatically
set
> by the VM at the time the error is first thrown.  If it is later caught and
> re-thrown, it is not changed.  This makes it easier to track down the source
of
> errors when they occur.

I generally use strings for internal errors. We have API to preserve stack
traces, so it shouldn't be necessary to use subclasses of Error. This works both
for futures and rethrow.

However, in this case, I'd suggest using "internalError" from ../errors.dart, as
I'd like to keep the code free of "throw" in preference of using API like what's
in error.dart. internalError adds context to errors, and this has been helpful
for collecting crash reports.

[danrubel, 2017-03-21 15:40:52]

Description was changed from

==========
fasta.CommentToken implement analyzer.CommentToken

* update the fasta scanner to generate comment and dartdoc tokens

* replace fasta.preceedingComments field
  with preceedingCommentTokens field and preceedingComments getter/setter

* update token translator
==========

to

==========
fasta.CommentToken implement analyzer.CommentToken

* update the fasta scanner to generate comment and dartdoc tokens

* replace fasta.preceedingComments field
  with preceedingCommentTokens field and preceedingComments getter/setter

* update token translator

R=paulberry@google.com

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

[danrubel, 2017-03-21 15:40:54]

Committed patchset #1 (id:1) manually as
80c8479587cfd0cef91057fe5e66c54ed9e19f38 (presubmit successful).

[ahe, 2017-03-22 09:44:26]

lgtm

[danrubel, 2017-03-22 17:10:49]

https://codereview.chromium.org/2763833002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/scanner/token.dart (right):

https://codereview.chromium.org/2763833002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/scanner/token.dart:457: throw 'unsupported
operation';
On 2017/03/21 14:36:50, ahe wrote:
> On 2017/03/21 14:23:07, Paul Berry wrote:
> > Change this to `throw new StateError('unsupported operation');`
> > 
> > Rationale: Error objects contain a stackTrace member which is automatically
> set
> > by the VM at the time the error is first thrown.  If it is later caught and
> > re-thrown, it is not changed.  This makes it easier to track down the source
> of
> > errors when they occur.
> 
> I generally use strings for internal errors. We have API to preserve stack
> traces, so it shouldn't be necessary to use subclasses of Error. This works
both
> for futures and rethrow.
> 
> However, in this case, I'd suggest using "internalError" from ../errors.dart,
as
> I'd like to keep the code free of "throw" in preference of using API like
what's
> in error.dart. internalError adds context to errors, and this has been helpful
> for collecting crash reports.

Fixed. https://codereview.chromium.org/2767083002/

https://codereview.chromium.org/2763833002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/scanner/token.dart:462: // TODO: implement remove
On 2017/03/21 14:23:07, Paul Berry wrote:
> Nit: change this to TODO(danrubel)

Fixed. https://codereview.chromium.org/2767083002/

https://codereview.chromium.org/2763833002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/scanner/token.dart:463: throw 'not implemented yet';
On 2017/03/21 14:23:07, Paul Berry wrote:
> Similarly, change to `throw new UnimplementedError();`

Fixed. https://codereview.chromium.org/2767083002/