-
Notifications
You must be signed in to change notification settings - Fork 226
Update the declaring constructors feature specification #4524
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for making these changes.
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
accepted/future-releases/primary-constructors/feature-specification.md
Outdated
Show resolved
Hide resolved
has the name `C`. If it exists, _D2_ omits the part derived from | ||
`'.' <identifierOrNew>` that follows the name and type parameter list, if | ||
any, in _D_. Moreover, _D2_ omits the formal parameter list _L_ that | ||
follows the name, type parameter list, if any, and `.id`, if any. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The last sentence is confusing. Does it omit the type parameter list? (I know how to read it, it's just not my first reading of that sentence. Maybe stop after _L_
. Maybe define _L_
first. Let _L_ be the formal parameter list that follows the name, type parameter list (if any) and
.id (if any).
. Then you can omit it, when it's clear what it is.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great review comments, thanks!
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
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
then _k2_ has an initializer list with the same elements in the same order. | ||
|
||
Finally, _k_ is added to _D2_, and _D_ is replaced by _D2_. | ||
Finally, _k2_ is added to _D2_, and _D_ is replaced by _D2_. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced in what? ("Everywhere", probably. Any and every reference to D is now a reference to D2 instead, there is no D.)
When does this replacement happen, precisely?
-
After parsing, for sure.
-
And after all transitivie file dependencies are parsed, probably.
-
After top-level name resolution and export scope resolution, probably.
-
After checking type hierarchy validity? (No cycles, no extending a non-class.) Probably. This change does not affect - type hierarchy relations, can be before or after.
-
After checking class
interface
/base
/final
modifier rules. Again, the rewrite doesn't change anything, so can be after. -
After rewriting the superclass, if necessary? Probably, since it refers to superclass getters declarations, and those should not
-
Before checking if a
mixin class
has a non-trivial constructor. Since it doesn't specify when a primary constructor is trivial, it must be checked after desugaring. -
Before identifier resolution in members. The name of the new instance variables need to be in scope for that.
-
Before checking if class members are valid overrides of superinterface members.
-
Before checking if classes have duplicate member names. The rewrite adds declartions which should not have name clashes.
-
Before type inference.
That means that some parts of the language semantics see the original syntax, and some parts of the langauge semantics see the rewritten syntax.
And every part of the semantics must know and understand which one it sees, and what that means. Are we sure that's done correctly everywhere?
This PR corrects a couple of mistakes in the feature specification about declaring and primary constructors. It also introduces support for using
new
rather than the class name in declarations of non-declaring constructors (that is, all the kinds that we have today).