Create DeclaredSimpleIdentifier when parse a declaration with Fasta.

Create DeclaredSimpleIdentifier when parse a declaration with Fasta.

R=paulberry@google.com, ahe@google.com
BUG=

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

[Paul Berry, 2017-03-13 18:58:14]

lgtm

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

https://codereview.chromium.org/2749643002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/identifier_context.dart:135: const
IdentifierContext._('methodDeclaration', inDeclaration: true);
Are we going to have a bug with constructors, e.g.:

class C {
  C(); // (1)
  C.named(); // (2)
}

I think that when we hit the instances of "C" in (1) and (2), the parser will
treat them as "methodDeclaration" context, so they will be treated as "declared
identifiers".  But it seems like they shouldn't be, since they are just
references to the C declared above.

https://codereview.chromium.org/2749643002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/identifier_context.dart:248: /// Indicates
whether the identifier is a name in a declaration.
Suggestion: change this comment to "Indicates whether the identifier represents
a name which is being declared".  Rationale: a reader might be confused and
think that *all* identifiers appearing in a declaration have isDeclaration=true,
but really it's only the identifier that names the thing being declared.

[scheglov, 2017-03-13 19:54:49]

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

https://codereview.chromium.org/2749643002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/identifier_context.dart:135: const
IdentifierContext._('methodDeclaration', inDeclaration: true);
On 2017/03/13 18:58:14, Paul Berry wrote:
> Are we going to have a bug with constructors, e.g.:
> 
> class C {
>   C(); // (1)
>   C.named(); // (2)
> }
> 
> I think that when we hit the instances of "C" in (1) and (2), the parser will
> treat them as "methodDeclaration" context, so they will be treated as
"declared
> identifiers".  But it seems like they shouldn't be, since they are just
> references to the C declared above.

You're right, this was a bug for ast_builder.
I added checks into parser_test, so we verify it for both analyzer and fasta
parsers.
To fix for ast_builder we recreate SimpleIdentifier without isDeclaration flag.

https://codereview.chromium.org/2749643002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/identifier_context.dart:248: /// Indicates
whether the identifier is a name in a declaration.
On 2017/03/13 18:58:14, Paul Berry wrote:
> Suggestion: change this comment to "Indicates whether the identifier
represents
> a name which is being declared".  Rationale: a reader might be confused and
> think that *all* identifiers appearing in a declaration have
isDeclaration=true,
> but really it's only the identifier that names the thing being declared.

Done.
Thanks.

[scheglov, 2017-03-13 20:13:03]

Description was changed from

==========
Create DeclaredSimpleIdentifier when parse a declaration with Fasta.

R=ahe@google.com, paulberry@google.com
BUG=
==========

to

==========
Create DeclaredSimpleIdentifier when parse a declaration with Fasta.

R=paulberry@google.com, ahe@google.com
BUG=

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

[scheglov, 2017-03-13 20:13:04]

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