-
Notifications
You must be signed in to change notification settings - Fork 228
Specify that non-plain primary parameters are immutable #4575
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
…ons (but a primary parameter can still be modified in the body of a primary constructor)
…ons (but a primary parameter can still be modified in the body of a primary constructor)
|
Do we want this? Example of something you could do using side-effects: class StreamWrapper<T>([StreamController<T>? existingController]) {
final Stream<T> stream;
final StreamController<T> controller;
init:
controller = existingController ??= StreamController(),
stream = controller.stream;
}Initializer list entries are (probably) run after all initializing expressions, so asssignments only affect themselves and the body block. It may be a little confusing if we have: class C(int x) {
final x1 = x;
this: x3 = ++x;
final x2 = x;
final int x3;
}because the three references to Then don't do that. Put the |
|
@lrhn wrote:
Oh, you're right, we did not include non-declaring, non-initializing-formal, non-super parameters (we could call them plain), they can still be mutated also in initialization code. I adjusted the wording to match the decision. |
…s only, as decided by the language team
|
We also disallowed assigning to plain primary constructor parameters in instance variable initializer expressions, but not in initializer list expressions. So the text is still half right for those. |
[Edit: Corrected this paragraph] The decisions made at the meeting Nov 19 do not make this distinction, they disallow mutation of primary parameters (of any kind) in non-late instance variable initializing expressions and in the initializer list of the primary constructor body part. The rule you're proposing does make sense, though, so we should discuss it. The decision at the language meeting was rather strict. I think it would make sense to think about it one more time. One thing to note is that it would force a rewrite on declarations like the following: class A([int? i]) {
final int x = i ??= 42;
final int y = i + 1;
}This declaration would be an error because we're (possibly) assigning to class A([int? i]) {
final int x;
final int y;
this: x = i ??= 42, y = i + 1;
}This might improve the overall readability (look again, I'm not convinced). We might at least want the error message on the initialization of If a developer is navigating these considerations already then I tend to think that we might as well allow the first form above. We could have some kind of UI support ("hover on the initializing expression") to show the evaluation order of all the initializing expressions of a given class with a primary constructor. In any case, developers who are using side-effecting expressions in initializing expressions already need to be aware of the exact ordering of the evaluations. We should probably recommend an ordering which is aligned with the evaluation order: "Put the primary constructor body part after the instance variable declarations", or, at least, after the ones that are non-late and have an initializing expression. @dart-lang/language-team, WDYT? Should we make the first example above an error, and hence enforce something like the second form? I tend to prefer that we allow both forms because the improvement in readability by outlawing the first one isn't very convincing to me. [Edit: The PR has now been landed, and it uses the strictest ruleset -- primary parameters cannot be mutated in initialization code. So both of the above examples are now errors.] |
|
The update now also specifies an error for a double initialization of a non-final instance variable (in the initializing expression of the variable itself, as well as in the initializer list of the body part of a primary constructor ). |
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
| variable introduced by a declaring parameter is subject to override | ||
| inference, just like an explicitly declared instance variable.* | ||
| - if the combined member signature for a getter with the same name as `p` | ||
| from the superinterfaces of _D_ exists and has return type `T`, the |
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.
(Does not apply to extension type constructors.
They can have superinterfaces, but the representation variable is unrelated to those.)
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.
True, extension types are different in many ways. I also agree that override inference is unlikely to be a natural mechanism for an extension type. It is not hard to create an example where an extension type uses override inference, but it is completely contrived:
abstract class Cons {
Cons(this.x);
final int x;
Cons get next;
}
class ConsLink extends Cons {
final Cons next;
ConsLink(super.x, this.next);
}
class ConsLoop extends Cons {
Cons get next => this;
ConsLoop(super.x);
}
// The type of `next` is `Cons`, based on override inference.
extension type ExtendedCons(next) implements Cons {}
void main() {
final c0 = ConsLink(3, ConsLoop(4));
Cons c = c0;
for (int i = 0; i < 5; ++i) {
print(c.x);
c = c.next;
}
print('---');
ExtendedCons ec = .new(c0);
for (int i = 0; i < 5; ++ i) {
print(ec.x);
ec = ExtendedCons(ec.next);
}
}However, I don't really think it's useful to say "let's prevent override inference with extension types just because I can't come up with a good use case right now". Perhaps there will be a good use case at some point. In the meantime, it doesn't hurt anybody that override inference is never used with a representation variable.
Moving the treatment of extension types to a separate section might be a good idea, but it will also allow for accidental inconsistencies for properties which can be described just once rather than twice.
accepted/future-releases/primary-constructors/feature-specification.md
Outdated
Show resolved
Hide resolved
| of the derivation of the semantics of _D_. The derivation step will delete | ||
| elements that amount to the primary constructor. Semantically, it will add | ||
| a new constructor _k2_, and it will add zero or more instance variable | ||
| declarations. |
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.
Does not apply to extension type declarations, which must have a primary constructor declaration.
Maybe just move extension type primary constructors into its own section entirely, and don't try to hande it the same way as for the other type declarations, inline in the same sections where it's easy to forget a case that is also different.
Is there any way in which it's not different?
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 fact that an extension type must have a primary constructor is an unnecessary inconsistency. The handling of initialization of the representation variable, the static checking of accesses to the representation variable, in fact everything about the representation variable, is the same as for a final instance variable. It is irrational and error prone to insist that we must specify all these things from scratch. "DRY".
The only thing that differs is that the representation variable is stored in a different location (namely in the slot which is also considered to be the location where the reference to the extension typed entity is stored).
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.
Mostly phrasing nits, the ob-desugaring rant, and a suggestion to handle extension types completely separately. They differ so much that handling the exceptions in-line is bound to lead to some forgotten cases.
Only one correction related to the actual change, it forgot to say that you can't double-initialize using an initializing formal.
This PR specifies that it is a compile-time error to assign a value to a formal parameter declared by a primary constructor in the initializer list of the body part of the primary constructor, as well as in the initializing expression of a non-late instance variable declaration.