Eliminate in-body declaring constructors #4562
Conversation
|
This PR deals with the comments from @lrhn on an earlier PR #4543, and it updates the primary constructor feature specification to eliminate everything associated with in-body declaring constructors. The terminology now uses 'primary' everywhere, because that's the only new kind of constructor which is being added by this feature. I did not change the experimental flag because that's a transient and relatively hidden thing. |
|
https://dart-review.googlesource.com/c/sdk/+/460420 updates the spec parser to use the new grammar rules, which serves as a sanity check on the grammar. |
| have a body in the class body. | ||
| This document specifies _primary constructors_. This is a feature that allows | ||
| one constructor and a set of instance variables to be specified in a concise | ||
| form in the header of the declaration. Some elements can still be specified in |
There was a problem hiding this comment.
... of the class declaration ...
There is no context here, so "the declaration" feels like it should refer to either the constructor or one of the instance variables.
(I know it's not always a class, but the next sentence uses "class" too.)
Also maybe "Some elements" -> "Some constructor elements".
There was a problem hiding this comment.
Maybe instead of "Some elements ...":
If the primary constructor also needs an initializer list or body, those can be specified inside the class body.
| class header to remain simpler and more readable when there are many | ||
| parameters. The previous example would look as follows using a declaring | ||
| body constructor: | ||
| because it must be guaranteed that the primary constructor is executed on |
There was a problem hiding this comment.
Maybe "because it must be guaranteed" -> "to guarantee".
There was a problem hiding this comment.
Or even "This requirement must be upheld because it must be guaranteed" -> "This ensures"
| this(var int x, var int y); | ||
| } | ||
| ``` | ||
| In particular, every generative constructor in a declaration that has a |
There was a problem hiding this comment.
every other generative constructor
(than the primary constructor)
| parameters. The previous example would look as follows using a declaring | ||
| body constructor: | ||
| because it must be guaranteed that the primary constructor is executed on | ||
| every newly created instance of this class. |
There was a problem hiding this comment.
Why do we want to guarantee that?
(Because of the scope of instance variable initializers. Or "for reasons to be divulged later". Or something.)
| ``` | ||
| In particular, every generative constructor in a declaration that has a | ||
| primary constructor must be redirecting, and it must invoke the primary | ||
| constructor (directly or indirectly). This can be seen as a motivation for |
| *For an extension type, this ensures that the name and type of the | ||
| representation variable is well-defined, and existing rules about final | ||
| instance variables ensure that every other non-redirecting generative | ||
| constructor will initialize the representation variable. Moreover, there |
There was a problem hiding this comment.
(Are we sure there are such rules, and that they are not only defined for classes?)
|
|
||
| A compile-time error occurs if the name of the primary constructor is the | ||
| same as the name of a constructor (declaring or not) in the body. | ||
| same as the name of a constructor in the body. |
| compile-time error that we already have.* | ||
| *Moreover, it is an error if two constructor declarations in the body have | ||
| the same name. This is just restating a compile-time error that we already | ||
| have.* |
There was a problem hiding this comment.
And the primary constructor should also just be introducing a constructor with a name, and use the same rules as every other constructor declaration for avoiding conflicts.
Using special rules depending on the syntax being used is almost a guarantee to mis a case somewhere,
| body scope of _D_. Every primary parameter is entered into this scope. | ||
| The declaring parameter list of the primary constructor introduces a new | ||
| scope, the _primary initializer scope_, whose enclosing scope is the body | ||
| scope of _D_. Every primary parameter is entered into this scope. |
There was a problem hiding this comment.
What is a "primary parameter"?
(Is it just a parameter of the primary constructor? If so, say that.)
Is "entered into ... scope" a defined concept? I don't recognize the phrasing.
(But phrasing around scopes hasn't always been very consistent.)
Do I understand this correctly:
- The primary parameter scope is the parameter scope of the primary constructor, and the scope for the body.
- The primary initializer scope is the scope introduced for the initializer list, which also contains (final) bindings for the values of initializing formals and super parameters, and now it's also used for field initializers and contains the argument value of field parameters.
(The initializer scope could have the parameter scope as enclosing scope, and only add the extra parameters. Then nobody would start wondering whether assigning to a non-final parameter in the initializer list is visible in the body. It is, the two scopes contain the same variables, not just the same values.)
There was a problem hiding this comment.
I'm guessing this was a find/replace error from "declaring" -> "primary" and it should read "declaring parameter". :)
| inference, just like an explicitly declared instance variable.* | ||
| - otherwise, if `p` is optional and has a default value whose static type | ||
| in the empty context is a type `T` which is not `Null` then `p` has | ||
| declared type `T`. When `T` is `Null`, `p` has declared type `Object?`. |
There was a problem hiding this comment.
... p instead has declared type Object?.
(Otherwise we just gave p two different declared types.)
munificent
left a comment
munificent
left a comment
There was a problem hiding this comment.
LGTM! Thank you for your endless patience dealing with the churn in this proposal. Our users will be better off for it.
| have a body in the class body. | ||
| This document specifies _primary constructors_. This is a feature that allows | ||
| one constructor and a set of instance variables to be specified in a concise | ||
| form in the header of the declaration. Some elements can still be specified in |
There was a problem hiding this comment.
Maybe instead of "Some elements ...":
If the primary constructor also needs an initializer list or body, those can be specified inside the class body.
| class header to remain simpler and more readable when there are many | ||
| parameters. The previous example would look as follows using a declaring | ||
| body constructor: | ||
| because it must be guaranteed that the primary constructor is executed on |
There was a problem hiding this comment.
Or even "This requirement must be upheld because it must be guaranteed" -> "This ensures"
| three cases. | ||
|
|
||
| Moreover, we may get rid of all those occurrences of `required` in the | ||
| declaration itself, whereas the other version need to use an element in the |
There was a problem hiding this comment.
"version need" -> "version needs".
| initializer list of the constructor to initialize `z`. This is necessary | ||
| because `y` isn't in scope. Moreover, there cannot be other non-redirecting | ||
| generative constructors when there is a primary constructor, but in the | ||
| other version we could add another non-redirecting generative constructor |
There was a problem hiding this comment.
Maybe just "other version" -> "version without a primary constructor".
| <extensionTypeDeclaration> ::= // Modified rule. | ||
| 'extension' 'type' <classNameMaybePrimary> <interfaces>? | ||
| 'extension' 'type' <primaryConstructor> <interfaces>? |
There was a problem hiding this comment.
Given that extension types don't use final for the representation variable and we don't gain any more benefit from allowing the dead in-body declaring form... does it make sense to mention extension types in this proposal at all?
It seems to me that they are their own thing with their own syntax which happens to also be in the class header but is otherwise unrelated.
| | 'const' <constructorHead> <formalParameterList> // New form. | ||
| | <declaringConstantConstructorSignature>; | ||
| <primaryConstructorBodySignature> ::= // New rule. | ||
| 'this' initializers?; |
There was a problem hiding this comment.
Alternatively, get rid of all of the < and > on the non-terminals. Since terminals are always quoted, it's unambiguous to use bare identifiers for non-terminals.
| example, `factory() => C();` could be a method named `factory` with an | ||
| implicitly inferred return type, or it could be a factory constructor.* | ||
| implicitly inferred return type, or it could be a factory constructor whose | ||
| name is the name of the enclosing class.* |
There was a problem hiding this comment.
I find this really really confusing. I understand that the spec says a constructor's name includes the class name, but I think the spec is the only place where people use that terminology. I think it's much clearer to say that a constructor may be named or unnamed. Then here I would say an "unnamed factory constructor".
I'd love it if the spec reflected that notion too. It's hard to imagine any human deriving value from this line of the spec:
It is a compile-time error if the name of a constructor is not a constructor name.
| by `factory`, it proceeds to parse the following input as a factory | ||
| constructor. | ||
|
|
||
| *This is similar to how a statement starting with `switch` or `{` are |
| contain a declaring constructor that has exactly one declaring parameter | ||
| which is `final`. | ||
| If _D_ is an extension type, it is a compile-time error if the primary | ||
| constructor that _D_ contains does not have exactly one parameter. |
There was a problem hiding this comment.
If we don't treat extension types as having primary constructors, then this can be removed.
| body scope of _D_. Every primary parameter is entered into this scope. | ||
| The declaring parameter list of the primary constructor introduces a new | ||
| scope, the _primary initializer scope_, whose enclosing scope is the body | ||
| scope of _D_. Every primary parameter is entered into this scope. |
There was a problem hiding this comment.
I'm guessing this was a find/replace error from "declaring" -> "primary" and it should read "declaring parameter". :)
This PR eliminates the support for in-body declaring constructors in the feature specification about declaring constructors. It also changes the terminology to use 'primary' rather than 'declaring' about the newly introduced constructors, because all other kinds have been eliminated.