-
Notifications
You must be signed in to change notification settings - Fork 705
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
Clarify use of this
in initializing instance variables
#5310
Conversation
In my opinion there should be non - late final where I have made edits
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.
This change seems to reduce the coverage of the given text by removing some cases that should be included (namely: initialization of non-final instance variables).
src/language/classes.md
Outdated
@@ -175,10 +175,10 @@ Non-final instance variables and | |||
an implicit *setter* method. For details, | |||
check out [Getters and setters][]. | |||
|
|||
If you initialize a non-`late` instance variable where it's declared, | |||
If you initialize a non-`late` `final` instance variable where it's declared, |
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.
I'm not sure why you'd think that this doesn't initialize i
:
class A {
int i = 1;
}
void main() {
print(A().i); // '1'.
}
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.
Respected,
I am talking about non-late but final instance variable which is initialized at declaration time. In that case "this" keyword will not be accessible.
class Point{
final double a;
final double b;
//initialized at declaration
final double c=2.7;
Point(this.a,this.b,this.c);
}
In this case "this.c" will show error. Because it is already initialized.
But if "c" is not final then you can reassign it a new value.
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.
I am talking about non-late but final instance variable
OK, but the paragraph you're referring to is about access to this
in an initializing expression (which is the expression e
that may occur after =
in a variable declaration), and it is true for final as well as non-final variables that the initializing expression doesn't have access to this
. (Also, the given explanation is valid for both kinds of variables.)
It's a bit like seeing "You can't go faster than 25MPH here!" and suggesting that we should change it to "You can't go faster than 25MPH here on rainy days!". If the first sentence is true then the second one is true, too, but it isn't helpful to introduce a reduction of scope when the statement is true for the original, broader scope. It indirectly implies that the original statement wasn't true, but that's incorrect: It was true.
Same structure here: The fact that there is no access to this
in an initializing expression is true for all non-late instance variables, not just for final ones.
[Edit: added the word 'non-late' near the end.]
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.
If I understand correctly, @pdhankhar61 your issue is with the existing statement:
non-late instance variable initializers can’t access this.
And you're saying that statement is incorrect because you tried accessing this
for the non-late instance variables a
, b
and c
, and it works:
class Point{
double a;
double b;
//initialized at declaration
double c=2.7;
Point(this.a,this.b,this.c); // no errors, accessing `this` DOES work even though c is already initialized
}
But you found that statement is true if the variable is final
, so you're updating the statement to specify final
?
class Point{
final double a;
final double b;
//initialized at declaration
final double c=2.7;
Point(this.a,this.b,this.c); // error - 'c' is final and was given a value when it was declared, so it can't be set to a new value.
}
I could be totally missing the point of the section, but I'm finding it confusing too. It also doesn't seem like the example code starting on line 183 is related to the paragraph immediately preceding it (the same paragraph in question), which makes it even more confusing.
@eernstg (or anyone) So it's not just about initialization of non-late instance variables in general, but specifically that the paragraph makes it sound like you can't use this
to initialize non-late instance variables -- why does it say that doesn't work?
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.
Here is what it means that the initializing expression of a non-late instance variable does not have access to this:
class Point {
double a = this.b; // (1)
double b;
Point(this.b); // (2)
}
At (1), we have the initializing expression this.b
. We could have written a plain b
, because that's just a concise notation for the same thing. In both cases it is a compile-time error to have such an initializing expression, because "that location does not have access to this
".
The point is that the initializing expression e
of a non-late instance variable is evaluated before the object under construction is complete, so we can't allow any user-written code (like e
) to see the object, yet.
If the variable is late
then it can also have an initializing expression, and in this case it does have access to this
, because it's only evaluated later, when the object is complete. So late double a = this.b;
is fine.
However, the occurrence of this
at (2) is completely different: That's a declaration of a formal parameter of the constructor named Point
, and this particular kind of parameter is known as an 'initializing formal'.
The syntax of an initializing formal is roughly <type>? 'this' '.' <identifier>
, and it works as follows: The type <type>
is usually omitted (the ?
is there to indicate that we can omit it), so let's ignore that. It's an error if this declaration is a formal parameter of anything other than a non-redirecting generative constructor. The identifier must be the name of an instance variable of the enclosing class / mixin class / enum. Finally, at run time it causes the given instance variable to be initialized to the actual argument which is passed for that parameter.
So this.b
at (2) is not an expression, and it doesn't allow us to write any code that accesses the object at a point in time where it is incomplete, it's just a "magic word" which indicates that this formal parameter is an initializing formal. The constructor could also have been written as Point(double b) : this.b = b;
, so the magic word this
is just a request for this particular kind of syntactic sugar. The syntax this.b = b
is in turn equally magic: The occurrence of this.b
is not a general expression, it's a special syntax that allows us to specify that this particular element in the initializer list is used to initialize the instance variable named b
. (We could omit this
again). The point is again that initialization of the state of the object must take place under very controlled circumstances, because we cannot let user-written code access the object before it is complete. That's what all those "not a general expression" incantations are about.
In particular, 'access to this
' has nothing to do with initializing formals, in spite of the fact that the latter uses this
as part of the syntax.
But it might be helpful to say, somewhere, that an initializing formal parameter isn't an "access to this
" because it isn't an expression.
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.
Okay Now, I have understood you point @eernstg sir. Like that statement was confusing without example. I think you should add the example for that statement.
Delay in response is because of github, because it sends notifications with a delay.
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.
Thanks, @pdhankhar61!
So what have we learned here? Taking a look at the section as a whole, the following comes to mind:
I think it's confusing that this section says 'All uninitialized instance variables have the value null
', and it does not even mention that for many instance variables (namely the ones whose declared type is potentially non-nullable) it is simply a compile-time error if they are left uninitialized.
If we mention this then it also makes sense to mention the error, perhaps right after "can't access this
":
double initialX = 1.5;
class Point {
double? x = initialX; // OK, can access declarations that do not depend on `this`.
double? y = this.x; // Compile-time error, cannot access `this`.
}
@MaryaBelanger, I understand that this kind of documentation should be extremely succinct and to the point, but what do you think about adding something like this?
We should probably also mention somewhere that an initializing formal isn't an expression and hence it isn't an "access to this
" even in the case where it's written literally as this.x
, but that might be better placed in a section where initializing formals are introduced (the same section does use one initializing formal, but only refers to it as 'a constructor parameter', so perhaps initializing formals aren't described here at all).
Respected,
I am talking about non-late but final instance variable which is
initialized at declaration time. In that case "this" keyword will not be
accessible.
class Point{
final double a;
final double b;
//initialized at declaration
final double c=2.7;
Point(this.a,this.b,this.c);
}
In this case "this.c" will show error. Because it is already initialized.
But if "c" is not final then you can reassign it a new value.
Prince Kumar Dhankhar
…On Thu 2 Nov, 2023, 7:03 PM Erik Ernst, ***@***.***> wrote:
***@***.**** commented on this pull request.
This change seems to reduce the coverage of the given text by removing
some cases that should be included (namely: initialization of non-final
instance variables).
------------------------------
In src/language/classes.md
<#5310 (comment)>:
> @@ -175,10 +175,10 @@ Non-final instance variables and
an implicit *setter* method. For details,
check out [Getters and setters][].
-If you initialize a non-`late` instance variable where it's declared,
+If you initialize a non-`late` `final` instance variable where it's declared,
I'm not sure why you'd think that this doesn't initialize i:
class A {
int i = 1;
}
void main() {
print(A().i); // '1'.
}
—
Reply to this email directly, view it on GitHub
<#5310 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ARDMNAYOWRG2UZCPQW7ZG33YCOOJXAVCNFSM6AAAAAA62JGLEKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMJQGI3TCNJSGQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
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.
I think the comments I wrote a few days ago might not have been sent yet. (The behavior of Github reviewing isn't always obvious. Maybe this helps. ;-)
src/language/classes.md
Outdated
@@ -175,10 +175,10 @@ Non-final instance variables and | |||
an implicit *setter* method. For details, | |||
check out [Getters and setters][]. | |||
|
|||
If you initialize a non-`late` instance variable where it's declared, | |||
If you initialize a non-`late` `final` instance variable where it's declared, |
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.
I am talking about non-late but final instance variable
OK, but the paragraph you're referring to is about access to this
in an initializing expression (which is the expression e
that may occur after =
in a variable declaration), and it is true for final as well as non-final variables that the initializing expression doesn't have access to this
. (Also, the given explanation is valid for both kinds of variables.)
It's a bit like seeing "You can't go faster than 25MPH here!" and suggesting that we should change it to "You can't go faster than 25MPH here on rainy days!". If the first sentence is true then the second one is true, too, but it isn't helpful to introduce a reduction of scope when the statement is true for the original, broader scope. It indirectly implies that the original statement wasn't true, but that's incorrect: It was true.
Same structure here: The fact that there is no access to this
in an initializing expression is true for all non-late instance variables, not just for final ones.
[Edit: added the word 'non-late' near the end.]
src/language/classes.md
Outdated
@@ -175,10 +175,10 @@ Non-final instance variables and | |||
an implicit *setter* method. For details, | |||
check out [Getters and setters][]. | |||
|
|||
If you initialize a non-`late` instance variable where it's declared, | |||
If you initialize a non-`late` `final` instance variable where it's declared, |
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.
Here is what it means that the initializing expression of a non-late instance variable does not have access to this:
class Point {
double a = this.b; // (1)
double b;
Point(this.b); // (2)
}
At (1), we have the initializing expression this.b
. We could have written a plain b
, because that's just a concise notation for the same thing. In both cases it is a compile-time error to have such an initializing expression, because "that location does not have access to this
".
The point is that the initializing expression e
of a non-late instance variable is evaluated before the object under construction is complete, so we can't allow any user-written code (like e
) to see the object, yet.
If the variable is late
then it can also have an initializing expression, and in this case it does have access to this
, because it's only evaluated later, when the object is complete. So late double a = this.b;
is fine.
However, the occurrence of this
at (2) is completely different: That's a declaration of a formal parameter of the constructor named Point
, and this particular kind of parameter is known as an 'initializing formal'.
The syntax of an initializing formal is roughly <type>? 'this' '.' <identifier>
, and it works as follows: The type <type>
is usually omitted (the ?
is there to indicate that we can omit it), so let's ignore that. It's an error if this declaration is a formal parameter of anything other than a non-redirecting generative constructor. The identifier must be the name of an instance variable of the enclosing class / mixin class / enum. Finally, at run time it causes the given instance variable to be initialized to the actual argument which is passed for that parameter.
So this.b
at (2) is not an expression, and it doesn't allow us to write any code that accesses the object at a point in time where it is incomplete, it's just a "magic word" which indicates that this formal parameter is an initializing formal. The constructor could also have been written as Point(double b) : this.b = b;
, so the magic word this
is just a request for this particular kind of syntactic sugar. The syntax this.b = b
is in turn equally magic: The occurrence of this.b
is not a general expression, it's a special syntax that allows us to specify that this particular element in the initializer list is used to initialize the instance variable named b
. (We could omit this
again). The point is again that initialization of the state of the object must take place under very controlled circumstances, because we cannot let user-written code access the object before it is complete. That's what all those "not a general expression" incantations are about.
In particular, 'access to this
' has nothing to do with initializing formals, in spite of the fact that the latter uses this
as part of the syntax.
But it might be helpful to say, somewhere, that an initializing formal parameter isn't an "access to this
" because it isn't an expression.
@MaryaBelanger, I unresolved the thread about line 178 and tried to summarize what we can learn from this discussion. What do you think would be the best way to proceed? |
@eernstg I think the section would benefit from more explicit explanation, and possibly code showing the thing it's saying you can't do, since that seems to be where we got confused. @pdhankhar61 I'm going to add some commits to this PR to incorporate @eernstg's explanation. I'll request both your reviews again when I'm done and hopefully we can land this with you as the original author. :) Thank you for bringing this to our attention! |
Sounds great, thank you!
Prince Kumar Dhankhar
…On Tue 14 Nov, 2023, 1:12 AM Marya, ***@***.***> wrote:
@eernstg <https://github.com/eernstg> I think the section would benefit
from more explicit explanation, and possibly code showing the thing it's
saying you *can't* do, since that seems to be where we got confused.
@pdhankhar61 <https://github.com/pdhankhar61> I'm going to add some
commits to this PR to incorporate @eernstg <https://github.com/eernstg>'s
explanation. I'll request both your reviews again when I'm done and
hopefully we can land this with you as the original author. :) Thank you
for bringing this to our attention!
—
Reply to this email directly, view it on GitHub
<#5310 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ARDMNA5LJXV2JUVY36IV6TDYEJZYVAVCNFSM6AAAAAA62JGLEKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBYHEYTMOBWHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hi @MaryaBelanger and @pdhankhar61, I'm not sure whether anyone is waiting for input from me, so let me just refer to a comment that I gave a while ago:
In short, I still think it's misleading to add the word 'final' in those two locations. |
Hi @eernstg, sorry for the delay! I did see that quote was your conclusion and I was just working on the rewrite myself. Thanks for the reminder, will be done shortly! |
Here's what I changed:
@eernstg please review the wording and code sample (mostly used your comments, but just to double check!) and @pdhankhar61 please let us know if you think the new code sample and wording is clearer than it was before, thank you! |
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.
LGTM!
Agreed!
Prince Kumar Dhankhar
…On Fri 29 Dec, 2023, 7:47 PM Erik Ernst, ***@***.***> wrote:
***@***.**** approved this pull request.
LGTM!
------------------------------
In examples/misc/lib/language_tour/classes/point_this.dart
<#5310 (comment)>:
> @@ -0,0 +1,16 @@
+// ignore_for_file: invalid_reference_to_this, unnecessary_this
+// #docregion this-late
+double initialX = 1.5;
+
+class Point {
+ double? x = initialX;
+ // OK, can access declarations that do not depend on `this`.
It is slightly surprising to me that the comment targets code in the
preceding line rather than the following line. I guess that's the preferred
style in this context?
------------------------------
In examples/misc/lib/language_tour/classes/point_this.dart
<#5310 (comment)>:
> @@ -0,0 +1,16 @@
+// ignore_for_file: invalid_reference_to_this, unnecessary_this
+// #docregion this-late
+double initialX = 1.5;
+
+class Point {
+ double? x = initialX;
+ // OK, can access declarations that do not depend on `this`.
+ double? y = this.x;
+ // ERROR, can't access `this` in non-`late` initializer.
+ late double? z = this.x;
+ // OK, can access `this` in `late` initializer.
+
+ Point(this.x, this.y);
+ // OK, not an initializing expression, can access `this`.
This one is difficult. ;-)
I think it's very useful to state explicitly that this.x and this.y are
not expressions at all, not even the otherwise plausible construct that we
call <assignableExpression> which is a specification of the target of an
assignment (occurring on the left hand side of =).
If we had considered each of them as the left hand side of an assignment
then we could be initializing an instance of a subclass of Point that
declares set x(int value) {...}, and in that case we'd have to call that
setter. That's not the actual semantics: this.x specifies that the actual
argument is stored in the instance variable x in the current class body,
it will never call an overriding setter. So this.x is not an expression
and it's not an assignable expression.
In order to make that point even more firmly I think it's safer to avoid
talking about "can access this" at all: that sounds a bit like "the
semantics of this.x includes expression evaluation, and during that
expression evaluation we can evaluate this, just like any other
expression". And we don't want the reader to think like that.
Perhaps we could say something like this:
⬇️ Suggested change
- // OK, not an initializing expression, can access `this`.
+ // OK, `this.someName` is a parameter declaration, not an expression.
------------------------------
In src/language/classes.md
<#5310 (comment)>:
> +<?code-excerpt "misc/lib/language_tour/classes/point_this.dart (this-late)"?>
+```dart
+double initialX = 1.5;
+
+class Point {
+ double? x = initialX;
+ // OK, can access declarations that do not depend on `this`.
+ double? y = this.x;
+ // ERROR, can't access `this` in non-`late` initializer.
+ late double? z = this.x;
+ // OK, can access `this` in `late` initializer.
+
+ Point(this.x, this.y);
+ // OK, not an initializing expression, can access `this`.
+}
+```
Similar comments as in
examples/misc/lib/language_tour/classes/point_this.dart. (I guess that
library and this snippet are kept in sync automatically).
------------------------------
In src/language/classes.md
<#5310 (comment)>:
> +Uninitialized instance variables declared with
+[nullable types][] have the value `null`.
I usually try to minimize the use of plural in situations like this,
because of the tendency to introduce ambiguities. This could also be a
matter of style/convention (it would be a bad idea to switch to singular if
this location would then stand out as an exception). Otherwise:
⬇️ Suggested change
-Uninitialized instance variables declared with
-[nullable types][] have the value `null`.
+An uninitialized instance variable declared with a
+[nullable type][] has the value `null`.
------------------------------
In src/language/classes.md
<#5310 (comment)>:
> @@ -350,3 +370,5 @@ can pass a static method as a parameter to a constant constructor.
[initializer list]: /language/constructors#initializer-list
[factory constructor]: /language/constructors#factory-constructors
[late-final-ivar]: /effective-dart/design#avoid-public-late-final-fields-without-initializers
+[nullable types]: /null-safety/understanding-null-safety#using-nullable-types
If the corresponding change was made above:
⬇️ Suggested change
-[nullable types]: /null-safety/understanding-null-safety#using-nullable-types
+[nullable type]: /null-safety/understanding-null-safety#using-nullable-types
—
Reply to this email directly, view it on GitHub
<#5310 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ARDMNA54LPRKW7PLJDPF5CTYL3GIJAVCNFSM6AAAAAA62JGLEKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOOJZGAYDQNRRG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Co-authored-by: Erik Ernst <eernst@google.com>
/gcbrun |
Visit the preview URL for this PR (updated for commit 2de12d3): |
this
in initializing instance variables
Thank you @pdhankhar61 and @MaryaBelanger for working on this!! I'm excited for the improvements to have landed :) |
Thanks to all of you too.
Prince Kumar Dhankhar
…On Thu 4 Jan, 2024, 1:09 AM Parker Lougheed, ***@***.***> wrote:
Thank you @pdhankhar61 <https://github.com/pdhankhar61> and @MaryaBelanger
<https://github.com/MaryaBelanger> for working on this!! I'm excited for
the improvements to have landed :)
—
Reply to this email directly, view it on GitHub
<#5310 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ARDMNA7DE4K4PMQMT6EFRFDYMWXWFAVCNFSM6AAAAAA62JGLEKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZVHA3TENJXGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
) Co-authored-by: Marya <111139605+MaryaBelanger@users.noreply.github.com> Co-authored-by: Marya Belanger <mbelanger@google.com> Co-authored-by: Erik Ernst <eernst@google.com> Co-authored-by: Eric Windmill <ewindmill@google.com> Co-authored-by: Eric Windmill <eric@ericwindmill.com> Co-authored-by: Parker Lougheed <parlough@gmail.com>
I found this conceptual bug or mistake. Checke here https://dart.dev/language/classes#instance-variables
This is original-
"If you initialize a non-late instance variable where it’s declared, the value is set when the instance is created, which is before the constructor and its initializer list execute. As a result, non-late instance variable initializers can’t access this."
Suggested Changes-
"If you initialize a non-late final instance variable where it’s declared, the value is set when the instance is created, which is before the constructor and its initializer list execute. As a result, non-late final instance variable initializers can’t access this."