Add AstBuilder support for DottedName and ConditionalUri.

Add AstBuilder support for DottedName and ConditionalUri.

R=scheglov@google.com

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

[Paul Berry, 2017-02-21 21:07:24]

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

[scheglov, 2017-02-21 21:32:14]

LGTM

https://codereview.chromium.org/2709623004/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/parser/listener.dart (right):

https://codereview.chromium.org/2709623004/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/parser/listener.dart:335: /// - Condition (literal
string; only present if [equalitySign is not `null`)
[equalitySign]

[Paul Berry, 2017-02-21 21:37:37]

https://codereview.chromium.org/2709623004/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/parser/listener.dart (right):

https://codereview.chromium.org/2709623004/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/parser/listener.dart:335: /// - Condition (literal
string; only present if [equalitySign is not `null`)
On 2017/02/21 21:32:14, scheglov wrote:
> [equalitySign]

Done.

[Paul Berry, 2017-02-21 21:43:12]

Description was changed from

==========
Add AstBuilder support for DottedName and ConditionalUri.
==========

to

==========
Add AstBuilder support for DottedName and ConditionalUri.

R=scheglov@google.com

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

[Paul Berry, 2017-02-21 21:43:13]

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

[ahe, 2017-02-22 05:58:53]

lgtm

https://codereview.chromium.org/2709623004/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart (right):

https://codereview.chromium.org/2709623004/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:745: // the listener.
That would be ifKeyword.next.endGroup.

https://codereview.chromium.org/2709623004/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:756: Token leftParen =
ifKeyword.next;
The scanner should have set up the token stream so that there's a synthetic
endGroup token for the right parenthesis.

The next token of the endGroup will be an error token. The parser may not
recover from this situation.

[Paul Berry, 2017-02-22 21:20:13]

https://codereview.chromium.org/2709623004/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart (right):

https://codereview.chromium.org/2709623004/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:756: Token leftParen =
ifKeyword.next;
On 2017/02/22 05:58:53, ahe wrote:
> The scanner should have set up the token stream so that there's a synthetic
> endGroup token for the right parenthesis.
> 
> The next token of the endGroup will be an error token. The parser may not
> recover from this situation.

I think you're answering the question "what if there is no ')' token due to an
error in the file being parsed?"  My question is "what if there is no '('
token?"

Currently, the parser calls expect() to consume the '(' token, and expect()
calls reportUnrecoverableError(), which calls handleUnrecoverableError(), which
throws an exception.  So the question is moot for now.

But in the long run we're going to have to change things so that when the parser
is invoked by analyzer, the error can be recovered from, at which point we're
going to have to decide what should happen here.  I don't know whether we'll
wind up introducing a synthetic '(' token or doing something else.  We'll have
to make sure completion does the right thing.

Leaving the code as is for now.

[ahe, 2017-02-23 17:51:52]

https://codereview.chromium.org/2709623004/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart (right):

https://codereview.chromium.org/2709623004/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:756: Token leftParen =
ifKeyword.next;
On 2017/02/22 21:20:13, Paul Berry wrote:
> On 2017/02/22 05:58:53, ahe wrote:
> > The scanner should have set up the token stream so that there's a synthetic
> > endGroup token for the right parenthesis.
> > 
> > The next token of the endGroup will be an error token. The parser may not
> > recover from this situation.
> 
> I think you're answering the question "what if there is no ')' token due to an
> error in the file being parsed?"  My question is "what if there is no '('
> token?"

Yes. I answered "what if there's no close parenthesis".
 
> Currently, the parser calls expect() to consume the '(' token, and expect()
> calls reportUnrecoverableError(), which calls handleUnrecoverableError(),
which
> throws an exception.  So the question is moot for now.

Yes, it is moot for now.

However, handleUnrecoverableError may recover. Take a look at handleError in
pkg/compiler/lib/src/parser/element_listener.dart.

> But in the long run we're going to have to change things so that when the
parser
> is invoked by analyzer, the error can be recovered from, at which point we're
> going to have to decide what should happen here.  I don't know whether we'll
> wind up introducing a synthetic '(' token or doing something else.  We'll have
> to make sure completion does the right thing.

I have been thinking that one should start by introducing a synthetic version of
whatever is missing, however, it's more complicated than that.

> Leaving the code as is for now.

Yes. I'm just using your todos as an opportunity to provide some additional
details.