Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[spec] Library version comments are not being ignored by the compiler. #2945

Open
modulovalue opened this issue Mar 24, 2023 · 10 comments
Open

Comments

@modulovalue
Copy link

modulovalue commented Mar 24, 2023

The language specification states that the content of a non-documentation comment must be ignored by the compiler:

Dart supports both single-line and multi-line comments.
A \Index{single line comment} begins with the token \code{//}.
Everything between \code{//} and the end of line
must be ignored by the Dart compiler
unless the comment is a documentation comment.

It looks to me like this is not consistent with the behavior of the "library version comment construct" e.g. // @dart = 2.12.

@lrhn
Copy link
Member

lrhn commented Mar 24, 2023

Supporting multiple versions of a language is inherently inconsistent.
And the compiler should also ignore documentation comments.

Think of it as every file having an metadata which includes the associated language-version.
The compiler uses that language version to decide how to parse and interpret the file.
The language version is pre-computed and fed to the compiler by a previous step, based on data not inherent to the file (like package_config.json, the path of the Dart file, and the native SDK language version) and also checks for a //@dart= version override in the file itself.

Then the compiler parses the file as the assigned language version, and it completely ignores comments while doing so.

@modulovalue
Copy link
Author

And the compiler should also ignore documentation comments.

Are you implying that:

  • the compiler should ignore documentation comments, but there are cases where it doesn't or
  • that the spec should say that documentation comments are also ignored by the compiler?

Are there plans to specify the version override comment in the language spec?

The analyzer says:

The language version override must be specified before any declaration or directive.
Try moving the language version override to the top of the file.

but there are exceptions to that brief description because such a language override comment needs to come after the optional hashbang. The following example is correct (according to the analyzer description), but it is not a valid Dart program.

// @dart=2.12
#!

This is highly pedantic (and very low priority, I am aware of that). My goal is just to eventually have a correct autogenerated parser, and I've noticed that this part might be underspecified.

@lrhn
Copy link
Member

lrhn commented Mar 24, 2023

This is underspecified because the language specification only specifies one language version.

The grammar in the specification is for that language version only. It doesn't mention features from older language versions which were removed. (So far, we've just been lucky to not remove much.)

The grammar for Dart 3.0 will have removed the ability to use : for default values of optional named parameters.
The compiler from the Dart 3.0 SDK will also be able to parse Dart 2.19 code using :, even if it's no longer in the language specification grammar at all.

So, the compiler is the sum of the languages for multiple Dart versions, each with its own grammar.
The language versioning marker (and other ways to version) is an extra-linguistic compiler feature used
to decide which language version the compiler will use for a specific file.

Because of that, the language version marker doesn't really fit into the specification of a single language version.

(Or, it's all a big hack. 😉)

The

Everything between \code{//} and the end of line 
must be ignored by the Dart compiler 
unless the comment is a documentation comment. 

is just meaningless.

The specification states what the source code means and doesn't mean.
If it assigns no meaning, and no errors, to comment content, then that's all there is to it.
Saying that the compiler "must ignore" something that the specification assigns no meaning to,
is like saying that readers of a book must ignore the whitespace between lines.

So, a compiler must not do anything, but if it does do something for syntactically valid comments, documentation or not, it's doing so because it wants to. Telling it that it mustn't isn't going to change anything.

@modulovalue
Copy link
Author

Thank you, I think I understand now the way that you are looking at this.

@munificent munificent closed this as not planned Won't fix, can't repro, duplicate, stale Mar 27, 2023
@modulovalue
Copy link
Author

@munificent you have closed this issue. This comment by Lasse says that he thinks that the compiler related statements are meaningless and I would agree (and depending on the POV they are inconsistent with the implementation).

Wouldn't it be best to remove those parts i.e. the following:

Everything between \code{//} and the end of line
must be ignored by the Dart compiler
unless the comment is a documentation comment.

Everything between \code{/}* and \code{*}/
must be ignored by the Dart compiler
unless the comment is a documentation comment.

@munificent
Copy link
Member

I closed it based on "I think I understand now the way that you are looking at this.". If you still think it's worth changing the spec, I'll re-open, but this hardly seems like it's worth spending time on to me.

@modulovalue
Copy link
Author

I left a comment here that contains a simple view that allows us to restore consistency with respect to this issue.

However, it's not as simple as it could (in my view, should) be, because Erik pointed out the following observation here:

We do have some test libraries where it occurs on line 10. This means that we would have to include support for parsing whitespace plus all kinds of comments (if not more) in this "version header", and we would artificially consider the "version language" to be different from Dart, even though it will have to duplicate all those grammar rules.

@eernstg
Copy link
Member

eernstg commented Jun 1, 2023

As @lrhn mentioned, each particular released version of the language specification and all the feature specifications should specify a single language version.

However, the most recent language specification specifies a few elements associated with null safety, and many, many other elements of null safety have not yet been integrated into the language specification (#2605 will do most of that).

This means that we don't have a complete lineup of officially authorized specifications of every single language version. However, the totality of the feature specifications and the language specification add up to the best approximation to the specification of the current language version that we have at this time.

In any case, each language processing tool (compilers, the analyzer, ...) has a number of implementation specific behaviors, and they are intentionally not mentioned in these specification documents (it only says, in a few locations, that this and that behavior is implementation specific; for instance, the binding of external function declarations to actual foreign language functions).

In particular, the choice of processing according to one out of multiple available language versions is an implementation specific behavior (strong hint: the language specification does not contain the string '@Dart' at all).

Each tool is allowed to do whatever it wants when it comes to implementation specific behavior, including choosing the language version based on finding @dart = 2.19 in a comment. It is also allowed to require that such version directives must occur before the first non-comment / non-whitespace token in the library or part, but none of that is part of the language.

I don't see any contradictions in this behavior: Language version selection is implementation specific behavior, but when the choice has been made, the processing of the program (static analysis and dynamic semantics) should conform to the specification of the relevant language versions (each library can have a separate language version).


Next, what does the language specification actually say about this?

The 'Comments' section in the language specification is ancient, and the part about "must be ignored" hasn't been changed since 2014 (ECMA-408). It should probably be rewritten extensively.

I think "must be ignored" should be taken to mean "assuming a specific language version (the spec doesn't care how that choice was made), the contents of comments will not change the diagnostic messages emitted by language processing tools, and it will not change the dynamic semantics of the program".

In other words, the Dart program ignores the comments, but tools can use them in whatever way they want.

The part about documentation comments and their scoping is highly underspecified, but that's probably OK. It doesn't make sense to insist that the behavior of DartDoc should be part of the language specification in full detail. At the same time, it does make sense to say that [someName] must be resolved according to a specific scope (because scopes are introduced and populated by Dart as a language). So the language specification just says what the scope is, and the DartDoc tool documentation should then explain when and how any particular name is looked up in said scope.


So we could use this issue as the location where such an update to the language specification is managed. I don't think it has to be a big effort.

@modulovalue
Copy link
Author

@eernstg I've opened a new issue #3124 as a response to your comment to keep this issue focused on language version overrides.

@modulovalue
Copy link
Author

Under the assumption that each specification describes a single version, and that the specification does not attempt to provide an exhaustive list of places where implementation specific behaviors can be expected, it seems to me now, that this issue can be considered as "working as intended".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants