Scopes for field initializer expressions with augmentations and primary constructors - #4729
Scopes for field initializer expressions with augmentations and primary constructors#4729lrhn wants to merge 3 commits into
Conversation
munificent
left a comment
munificent
left a comment
There was a problem hiding this comment.
I'm still really not a fan of having both the public and private names in scope for private named parameters. But I'm OK with iterating on that in issues and revising this later.
Some other suggestions but otherwise LGTM! :)
| ## Primary constructors | ||
|
|
||
| A `class`, `enum` or `extension type` declarationscan use the primary | ||
| A `class`, `enum` or `extension type` declarations can use the primary |
There was a problem hiding this comment.
"declarations can" -> "declaration can".
| * A complete instance variable declaration which is `late`, | ||
| or which is neither `final` nor `external`. | ||
| * A complete instance variable declaration with an initializer | ||
| expression which is not a potentially constant expression. |
There was a problem hiding this comment.
Is it not already an error to have a const constructor with fields that aren't final with potentially const initializers? What is this section adding to the language?
There was a problem hiding this comment.
I'd also say that these errors are the existing ones in pre-augmentations Dart plus the general principle that static analysis (including emission of compile-time errors) should be performed according to the semantic declarations, not according to one fragment at a time.
| the lexical scope for instance variable initializers is the class/enum body | ||
| scope. | ||
| At initialization time, for each non-`late` instance variable with | ||
| an initializer expression, in source order, evaluate the initializer expression |
There was a problem hiding this comment.
I don't think "in source" order is sufficiently precise to cover how instance variable initializers from different augmentations are ordered. Probably need to say something like:
At initialization time:
- For each declaration of the class or enum, in augmentation application order:
- For each non-
lateinstance variable in the declaration with an initializer expression, in source order:- ...
- For each non-
| * If, and only if, that class or enum declaration has a complete primary | ||
| constructor declaration which has one or more private named parameters | ||
| _(which are initializing formals or declaring parameters)_, then both | ||
| the private and the public names are in the field initializer scope. |
There was a problem hiding this comment.
I'm still not a fan of this approach.
There was a problem hiding this comment.
Same here. I'd prefer to treat the declaration using a private name consistently as having this private name for all lexical lookup operations. The only location where the corresponding public name is used is as the label of a named argument at a call site.
| an initializer expression, in source order, evaluate the initializer expression | ||
| in the runtime body scope, then initialize the variable to the result. | ||
|
|
||
| * If the class or enum has a primary constructor, each class or enum declaration |
There was a problem hiding this comment.
I think this would be a little clearer as:
* Otherwise (the class or enum has a primary constructor): Each class or enum declaration introduces...
| Then each non-`late` instance variable with an initializer expression in that | ||
| class or enum declaration, in source order, has its initializer expression | ||
| evaluated in that runtime field initializer scope, and the variable is | ||
| initialized to the result. |
There was a problem hiding this comment.
I don't like that both branches of the top-level if describe the same behavior here. I think it would be simpler if we organize it like:
1. Each class or enum declaration introduces a _field initializer scope_. This
is the lexical scope for instance variable initializers. The parent scope is
the body scope of the surrounding class or enum.
2. If the class or enum has a primary constructor:
1. For each parameter in combined constructor signature of the
constructor...
*[More specification here to populate the scope with the primary constructor
parameters...]*
*Otherwise if there is no primary constructor, the field initializer scope
is left empty.*
3. For each declaration of the class or enum, in augmentation application order:
1. For each non-`late` instance variable in the declaration with an
initializer expression, in source order:
1. Evaluate the initializer expression and initialize the variable
with the result.
This way, everything that can be left the same between both branches is shared.
eernstg
left a comment
eernstg
left a comment
There was a problem hiding this comment.
Looks good in general!
A couple of comments: I do think we need to choose an unambiguous terminology for the syntactic declarations of classes/enums/etc, and for the semantic declaration which is the result of applying all the augmenting declarations. Another question to consider is whether the corresponding public name should be in scope together with the private name, in the case of a private named parameter in a primary constructor.
| * A complete instance variable declaration which is `late`, | ||
| or which is neither `final` nor `external`. | ||
| * A complete instance variable declaration with an initializer | ||
| expression which is not a potentially constant expression. |
There was a problem hiding this comment.
I'd also say that these errors are the existing ones in pre-augmentations Dart plus the general principle that static analysis (including emission of compile-time errors) should be performed according to the semantic declarations, not according to one fragment at a time.
| the complete declaration, the actual implementation, of a primary constructor | ||
| might not be a primary constructor declaration._ | ||
| When invoking the initializing constructor with a valid argument list to | ||
| initialize a new object, perform instance variable initialization on |
There was a problem hiding this comment.
Just to be safe, we could spell out lateness here again:
| initialize a new object, perform instance variable initialization on | |
| initialize a new object, perform non-`late` instance variable initialization on |
| When invoking the initializing constructor with a valid argument list to | ||
| initialize a new object, perform instance variable initialization on | ||
| each declaration of the class or enum in source order, with that | ||
| given argument list. |
There was a problem hiding this comment.
'given' where? Perhaps there should be an extra sentence above introducing the invocation and in particular its actual argument list.
| * If, and only if, that class or enum declaration has a complete primary | ||
| constructor declaration which has one or more private named parameters | ||
| _(which are initializing formals or declaring parameters)_, then both | ||
| the private and the public names are in the field initializer scope. |
There was a problem hiding this comment.
Same here. I'd prefer to treat the declaration using a private name consistently as having this private name for all lexical lookup operations. The only location where the corresponding public name is used is as the label of a named argument at a call site.
| @@ -1286,45 +1295,111 @@ contain constructor declarations where: | |||
| #### Instance variable initialization during constructor invocation | |||
There was a problem hiding this comment.
We need to settle a terminology that clearly distinguishes the fragments that are represented as syntactic class/enum/... declarations from the semantic declarations that are obtained by applying all augmentations.
We could say 'declaration fragments' when we mean the syntactic declarations, and 'semantic declarations' when that's what we mean, but we really need to keep those two concepts strictly separated.
| * If the class or enum has a primary constructor, each class or enum declaration | ||
| introduces a _field initializer scope_ which: | ||
| * Has an entry for each parameter with a name in the combined constructor | ||
| signature of that constructor. |
There was a problem hiding this comment.
I don't think we have defined 'combined constructor signature'. It should suffice to talk about 'the constructor declaration' in 'the semantic class declaration', or just 'the semantic constructor declaration'.
One reason why we might want to avoid 'combined' here is that we already use 'combined member signature' to talk about the handling of non-identical member signatures in superinterfaces.
Another reason is that the semantic constructor declaration has a signature which isn't combined in the usual sense of that word, it is the semantic signature which is obtained by gathering pieces from the entire augmentation chain of the declaration, but those pieces are never allowed to disagree on anything, they are only allowed to omit certain elements (like the type or name), which will then be taken from other syntactic declarations.
| _If no declaration of a constructor has an explicit type, then a | ||
| type may have been inferred from a default value or it may have defaulted | ||
| to `dynamic`._ | ||
| * None of these entries are assignable. |
There was a problem hiding this comment.
This is not quite congruent with "a field initializer scope which:". Some other entries in this list have the same issue.
3 participants
Adds the "field initializer scope" which as the scope for instance variable initializers.
It's based on the names and types of the primary constructor's signature, and can be used whether there is a constructor or not. See: #4727