improve fasta export directive recovery

improve fasta export directive recovery

R=ahe@google.com

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

[danrubel, 2017-08-20 22:02:29]

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

[ahe, 2017-08-21 12:51:37]

I would like to discuss this in person.

https://codereview.chromium.org/3001993002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/parser/parser.dart (right):

https://codereview.chromium.org/3001993002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/parser.dart:1927: if (!identical(token.kind,
STRING_TOKEN)) {
Don't need to use identical on int.

https://codereview.chromium.org/3001993002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/parser.dart:1929: token =
listener.rewriter.insertTokenBefore(
Something is off here. If the parser does the rewriting, it doesn't make sense
that the listener holds the rewriter.

[danrubel, 2017-08-21 14:51:02]

https://codereview.chromium.org/3001993002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/parser/parser.dart (right):

https://codereview.chromium.org/3001993002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/parser.dart:1927: if (!identical(token.kind,
STRING_TOKEN)) {
On 2017/08/21 12:51:37, ahe wrote:
> Don't need to use identical on int.

Fair enough. I was just following what you already did on line 1945 below.

https://codereview.chromium.org/3001993002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/parser.dart:1929: token =
listener.rewriter.insertTokenBefore(
On 2017/08/21 12:51:37, ahe wrote:
> Something is off here. If the parser does the rewriting, it doesn't make sense
> that the listener holds the rewriter.

Yeah. I had the parser holding onto the rewriter but was asked to move it into
the listener. It makes sense for the parser to do the recovery because otherwise
various listeners will recover differently, which will produce different error
messages and different generated code for different listeners and confuse the
user.

[danrubel, 2017-08-22 11:39:58]

Revised as per discussion. PTAL.

[ahe, 2017-08-22 12:27:00]

lgtm

https://codereview.chromium.org/3001993002/diff/40001/pkg/compiler/lib/src/pa...
File pkg/compiler/lib/src/parser/element_listener.dart (right):

https://codereview.chromium.org/3001993002/diff/40001/pkg/compiler/lib/src/pa...
pkg/compiler/lib/src/parser/element_listener.dart:547: // TODO(danrubel) This
functionality is being replaced by
Add colon after "(danrubel)".

[ahe, 2017-08-22 12:28:12]

FWIW, I'm uneasy about having these additional fields in the parser. But moving
them to the listener isn't a great solution either.

[danrubel, 2017-08-22 20:52:30]

Description was changed from

==========
improve fasta export directive recovery
==========

to

==========
improve fasta export directive recovery

R=ahe@google.com

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

[danrubel, 2017-08-22 20:52:31]

Committed patchset #4 (id:60001) manually as
69e9106ead586f70303b15532cec374c7806f15d (presubmit successful).

[danrubel, 2017-08-25 15:30:18]

https://codereview.chromium.org/3001993002/diff/40001/pkg/compiler/lib/src/pa...
File pkg/compiler/lib/src/parser/element_listener.dart (right):

https://codereview.chromium.org/3001993002/diff/40001/pkg/compiler/lib/src/pa...
pkg/compiler/lib/src/parser/element_listener.dart:547: // TODO(danrubel) This
functionality is being replaced by
On 2017/08/22 12:27:00, ahe wrote:
> Add colon after "(danrubel)".

Fixed in https://codereview.chromium.org/3003263002/