Built-in identifier as prefix is not detected as a compile-time error. #28811

Open
eernstg opened this Issue Feb 17, 2017 · 5 comments

Projects

None yet

3 participants

@eernstg
Member
eernstg commented Feb 17, 2017 edited

[NB: Edited to reflect the fact that it is a compile-time error for a built-in identifier to be a prefix.]

Action items:

None of the tools flag the declaration of a built-in identifier as a prefix as a compile-time error, even though the language specification says so.

The language specification has been updated to outlaw built-in identifiers in type annotations as well as built-in identifiers as type annotations, which makes all the usages in the examples below compile-time errors as well (issue #28817). So tools should flag those usages as compile-time errors as well.

Background

The Dart tools differ in various ways when it comes to certain usages of built-in identifiers. I've explored one case, namely library prefixes which are built-in identifiers (which is an error according to the spec).

The vm generally treats built-in identifiers as keywords in these situations (in the sense that they are considered to mean the usual thing, e.g., abstract as a class modifier and not a user-defined name) and constructs using them in the ways I explored are simply syntax errors. Here's the behavior for variable declarations:

import 'dart:core' as abstract;
import 'dart:core' as as;
import 'dart:core' as deferred;
import 'dart:core' as dynamic;
import 'dart:core' as export;
import 'dart:core' as external;
import 'dart:core' as factory;
import 'dart:core' as get;
import 'dart:core' as implements;
import 'dart:core' as import;
import 'dart:core' as library;
import 'dart:core' as operator;
import 'dart:core' as part;
import 'dart:core' as set;
import 'dart:core' as static;
import 'dart:core' as typedef;

abstract.int v1 = 42; // unexpected token.
as.int v2 = 42; // unexpected token.
deferred.String v3 = '42';
dynamic.List v4 = [];
export.Map v5 = {}; // unexpected token.
external.double v6 = 3.14; // unexpected token.
factory.int v7 = 1; // unexpected token.
get.int v8 = 2; // accessor name expected.
implements.int v9 = 3; // unexpected token.
import.int v10 = 4; // unexpected token.
library.int v11 = 5; // unexpected token.
operator.int v12 = 6; // unexpected token.
part.int v13 = 7; // unexpected token.
set.int v14 = 8; // accessor name expected.
static.int v15 = 9; // unexpected token.
typedef.int v16 = 10; // type name expected.

class C {
  abstract.int v1 = 42; // unexpected token '.'.
  as.int v2 = 42; // unexpected token '.'.
  deferred.String v3 = '42';
  dynamic.List v4 = [];
  export.Map v5 = {}; // unexpected token '.'.
  external.double v6 = 3.14; // identifier expected.
  factory.int v7 = 1; // identifier expected.
  get.int v8 = 2; // identifier expected.
  implements.int v9 = 3; // unexpected token '.'.
  import.int v10 = 4; // unexpected token '.'.
  library.int v11 = 5; // unexpected token '.'.
  operator.int v12 = 6; // invalid operator overloading.
  part.int v13 = 7; // unexpected token '.'.
  set.int v14 = 8; // identifier expected.
  static.int v15 = 9; // identifier expected.
  typedef.int v16 = 10; // unexpected token.
}

main() {
  new C();
}

Skipping main, the imports and the cases with no errors, here are functions and instance methods:

abstract.int v1(abstract.int x) => x; // unexpected token.
as.int v2(as.int x) => x; // unexpected token.
export.Map v5(export.Map x) => x; // unexpected token.
external.double v6(external.double x) => x; // unexpected token.
factory.int v7(factory.int x) => x; // unexpected token.
get.int v8(get.int x) => x; // accessor name expected.
implements.int v9(implements.int x) => x; // unexpected token.
import.int v10(import.int x) => x; // unexpected token.
library.int v11(library.int x) => x; // unexpected token.
operator.int v12(operator.int x) => x; // unexpected token.
part.int v13(part.int x) => x; // unexpected token.
set.int v14(set.int x) => x; // accessor name expected.
static.int v15(static.int x) => x; // unexpected token.
typedef.int v16(typedef.int x) => x; // type name expected.

class C {
  abstract.int v1(abstract.int x) => x; // unexpected token '.'.
  export.Map v5(export.Map x) => x; // unexpected token '.'.
  external.double v6(external.double x) => x; // identifier expected.
  factory.int v7(factory.int x) => x; // identifier expected.
  get.int v8(get.int x) => x; // identifier expected.
  implements.int v9(implements.int x) => x; // unexpected token '.'.
  import.int v10(import.int x) => x; // unexpected token '.'.
  library.int v11(library.int x) => x; // unexpected token.
  operator.int v12(operator.int x) => x; // invalid operator overloading.
  part.int v13(part.int x) => x; // unexpected token '.'.
  set.int v14(set.int x) => x; // identifier expected.
  static.int v15(static.int x) => x; // identifier expected.
  typedef.int v16(typedef.int x) => x; // unexpected token '.'.
}

With dart2js the same constructs are flagged as syntax errors, that is, all of them except deferred and dynamic.

The analyzer handles all of them except typedef inside a class, where we get 6 messages for a single declaration:

class C {
  // Typedefs can't be declared inside classes.
  // Expected an identifier.
  // Typedefs must have an explicit list of parameters.
  // Unexpected text '.'
  // Expected a class member.
  // Undefined class 'int'.
  typedef.int v16 = 10;
}

dartdevc behaves like the analyzer, possibly running the same code.

Function declarations using these type expressions as a return type and for a parameter are essentially treated the same as the variable declarations by the vm and dart2js (everything except dynamic and deferred is considered as a syntax error), and the analyzer (only typedef in a class causes diagnostic messages of any kind).

Searching a substantial amount of existing code, there are no occurrences of any of these imports except for a test file built_in_identifier_prefix_test.dart which is used by analyzer, dev_compiler, and language_strong, and a single occurrence of the following:

  • import 'deferred_load.dart' as deferred;
  • import 'library.dart' as library;
@eernstg eernstg added the Type: meta label Feb 17, 2017
@eernstg
Member
eernstg commented Feb 17, 2017

Note the similarities between this issue and another issue which was just submitted: That one is about built-in identifiers as names of functions.

@alexander-doroshko

Probably related (built-in identifiers as var names): #24375

@eernstg
Member
eernstg commented Feb 17, 2017

Turns out I was mislead by one of the first things I found when exploring this area: built_in_identifier_prefix_test.dart, and of course a too sloppy spec search. The spec (Sect. Identifier Reference) does in fact indicate that it is a compile-time error:

It is a compile-time error if a built-in identifier is used as the declared name of a
prefix, class, type parameter or type alias. It is a compile-time error to use a
built-in identifier other than dynamic as a type annotation or type parameter.

So the remaining issue is that none of the tools raise the required compile-time error for the import clauses where built-in identifiers are used as declared names of prefixes.

As Matthias mentions in issue #6970, the spec may need to be adjusted to prohibit abstract<int> and similar cases where the built-in identifier occurs in (not as) a type annotation. This change should also outlaw things like abstract.C, which would make all the usages in my examples compile-time errors as well (which is what the tools mostly do already).

Adjusted the title and the issue accordingly.

@eernstg eernstg changed the title from Treatment of built-in identifier as prefix is inconsistent. to Built-in identifier as prefix is not detected as a compile-time error. Feb 17, 2017
@lrhn
Member
lrhn commented Feb 17, 2017

So the spec prohibits any way to create a type named by a built-in identifier, and any use as a prefix.
That means that a built-in identifier is never a type (plain or generic), and I guess that was intended to suppress any use like abstract<int> - because abstract is not a type, that has to be an error.

However, we probably want it to parse differently than as a malformed parameterized type, so we also need to say something to the effect that the type production must not start with a built-in identifier. - or something more complex depending on how we want to parse prefix.abstract.
It would prevent both abstract<int> and List<abstract> because the type argument is itself using the type production.

@eernstg
Member
eernstg commented Feb 17, 2017

.. the type production must not start with a built-in identifier.

That is already part of the grammar in https://codereview.chromium.org/2688903004/, and it eliminates a large number of ambiguities.

@eernstg eernstg added the Type: bug label Feb 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment