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. |
accepted/future-releases/primary-constructors/feature-specification.md
Outdated
Show resolved
Hide resolved
accepted/future-releases/primary-constructors/feature-specification.md
Outdated
Show resolved
Hide resolved
accepted/future-releases/primary-constructors/feature-specification.md
Outdated
Show resolved
Hide resolved
accepted/future-releases/primary-constructors/feature-specification.md
Outdated
Show resolved
Hide resolved
accepted/future-releases/primary-constructors/feature-specification.md
Outdated
Show resolved
Hide resolved
| *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?)
accepted/future-releases/primary-constructors/feature-specification.md
Outdated
Show resolved
Hide resolved
accepted/future-releases/primary-constructors/feature-specification.md
Outdated
Show resolved
Hide resolved
accepted/future-releases/primary-constructors/feature-specification.md
Outdated
Show resolved
Hide resolved
accepted/future-releases/primary-constructors/feature-specification.md
Outdated
Show resolved
Hide resolved
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.
accepted/future-releases/primary-constructors/feature-specification.md
Outdated
Show resolved
Hide resolved
accepted/future-releases/primary-constructors/feature-specification.md
Outdated
Show resolved
Hide resolved
accepted/future-releases/primary-constructors/feature-specification.md
Outdated
Show resolved
Hide resolved
accepted/future-releases/primary-constructors/feature-specification.md
Outdated
Show resolved
Hide resolved
| <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.
There was a problem hiding this comment.
We do have to mention them because it is a special exception that an extension type can have multiple generative non-redirecting constructors. That's again needed in order to have multiple arguments in a constructor, and it would be an unnecessary breaking change to take that away.
There was a problem hiding this comment.
Also, the syntax for the primary constructor is the same (all the elements of the primary constructor can be used, including the modifier final on the parameter, the ability to make the parameter named and/or optional, possibly with a default value; and we can have a constructor body and assertions). I don't think we should make a bunch of exceptions for extension types, just so they can't to a number of things that other primary constructors can do.
accepted/future-releases/primary-constructors/feature-specification.md
Outdated
Show resolved
Hide resolved
| 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.
There was a problem hiding this comment.
I've seen that again and again recently. Everybody else seems to think that it's OK for a declaration to have the empty name. I think that's similar to a division by zero, and I'm sure there will be cases where we get into absurd discussions about whether or not the empty name occurs at a specific location in the program. ;-)
I'll try to think about ways to allow a different terminology, but right now I'll stick to the style which has always been used in the language specification (and, I believe, in most feature specifications as well.)
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.