Complain about built-in identifiers.

Complain about built-in identifiers.

R=johnniwinther@google.com, sigmund@google.com

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

[ahe, 2017-03-14 15:03:56]

This isn't ready for review yet: I'll have to make some changes to dart2js to
ignore these error codes.

I don't think this conflicts with what Dan is currently working, but it is
related, I think.

[danrubel, 2017-03-14 15:21:42]

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

[danrubel, 2017-03-14 15:21:43]

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

https://codereview.chromium.org/2750863002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/scanner/token.dart:180: return keyword.isBuiltIn ||
identical("deferred", lexeme);
Similar to what Paul recommended in the fasta.Keyword CL (see link below), I
suggest we add the following comment here.

// TODO: remove `identical("deferred", lexeme)` once dartbug.com/29069 is fixed.


https://codereview.chromium.org/2746023003/diff/40001/pkg/front_end/lib/src/f...

[ahe, 2017-03-14 15:24:36]

Thank you, Dan!

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

https://codereview.chromium.org/2750863002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/scanner/token.dart:180: return keyword.isBuiltIn ||
identical("deferred", lexeme);
On 2017/03/14 15:21:43, danrubel wrote:
> Similar to what Paul recommended in the fasta.Keyword CL.

Yes. That's how I knew about this case :-)

I'll add the todo.

[Siggi Cherem (dart-lang), 2017-03-14 16:04:44]

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

[Siggi Cherem (dart-lang), 2017-03-14 16:04:45]

lgtm

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

https://codereview.chromium.org/2750863002/diff/60001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/parser/parser.dart:908: if (!optional("dynamic",
token)) {
+floitsch

just a heads up: this will need to be changed on the `void` CL (that makes
`void` a built-in and allows it as a type everywhere) or here if Florian lands
that change first.

[Paul Berry, 2017-03-14 19:57:56]

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

[Paul Berry, 2017-03-14 19:57:57]

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

https://codereview.chromium.org/2750863002/diff/60001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/parser/parser.dart:895: switch (context) {
Rather than put a switch statement here (which can be slow due to the fact that
it typically gets implemented as a sequence of equality checks), consider adding
a "disallowsBuiltInIdentifiers" field to the IdentifierContext class.  That way
this code turns into something like:

if (context.disallowsBuiltInIdentifiers) {
  if (context.inDeclaration) {
    reportRecoverableError(
        token, ErrorKind.BuiltInIdentifierInDeclaration);
  } else if (!optional("dynamic", token)) {
    reportRecoverableError(
        token, ErrorKind.BuiltInIdentifierAsType);
  }
}

[ahe, 2017-03-16 09:10:49]

ahe@google.com changed reviewers:
+ johnniwinther@google.com, karlklose@google.com

[ahe, 2017-03-16 09:10:51]

Karl and Johnni, this is now ready for review, could you take a look?

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

https://codereview.chromium.org/2750863002/diff/60001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/parser/parser.dart:895: switch (context) {
On 2017/03/14 19:57:57, Paul Berry wrote:
> Rather than put a switch statement here (which can be slow due to the fact
that
> it typically gets implemented as a sequence of equality checks), consider
adding
> a "disallowsBuiltInIdentifiers" field to the IdentifierContext class.  That
way
> this code turns into something like:
> 
> if (context.disallowsBuiltInIdentifiers) {
>   if (context.inDeclaration) {
>     reportRecoverableError(
>         token, ErrorKind.BuiltInIdentifierInDeclaration);
>   } else if (!optional("dynamic", token)) {
>     reportRecoverableError(
>         token, ErrorKind.BuiltInIdentifierAsType);
>   }
> }

Done.

https://codereview.chromium.org/2750863002/diff/60001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/parser/parser.dart:908: if (!optional("dynamic",
token)) {
On 2017/03/14 16:04:45, Siggi Cherem (dart-lang) wrote:
> +floitsch
> 
> just a heads up: this will need to be changed on the `void` CL (that makes
> `void` a built-in and allows it as a type everywhere) or here if Florian lands
> that change first.

Acknowledged.

[Johnni Winther, 2017-03-16 09:22:40]

lgtm

[ahe, 2017-03-16 10:01:34]

Description was changed from

==========
Complain about built-in identifiers.
==========

to

==========
Complain about built-in identifiers.

R=johnniwinther@google.com, sigmund@google.com

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

[ahe, 2017-03-16 10:01:36]

Committed patchset #6 (id:100001) manually as
c3330b140775cf8d2e2b42fc08dc4fef5ff85cf1 (presubmit successful).