Skip to content
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

Merged
merged 17 commits into from
Jan 3, 2024

Conversation

pdhankhar61
Copy link
Contributor

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."

In my opinion there should be non - late final where I have made edits
Copy link
Member

@eernstg eernstg left a 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).

@@ -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,
Copy link
Member

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'.
}

Copy link
Contributor Author

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.

Copy link
Member

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.]

Copy link
Contributor

@MaryaBelanger MaryaBelanger Nov 4, 2023

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?

Copy link
Member

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.

Copy link
Contributor Author

@pdhankhar61 pdhankhar61 Nov 8, 2023

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.

Copy link
Member

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).

@pdhankhar61
Copy link
Contributor Author

pdhankhar61 commented Nov 2, 2023 via email

@atsansone atsansone added the review.tech Awaiting Technical Review label Nov 7, 2023
Copy link
Member

@eernstg eernstg left a 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. ;-)

@@ -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,
Copy link
Member

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.]

@@ -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,
Copy link
Member

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.

@eernstg
Copy link
Member

eernstg commented Nov 8, 2023

@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?

@MaryaBelanger
Copy link
Contributor

@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!

@pdhankhar61
Copy link
Contributor Author

pdhankhar61 commented Nov 14, 2023 via email

@MaryaBelanger MaryaBelanger added review.copy Awaiting Copy Review and removed review.tech Awaiting Technical Review labels Nov 21, 2023
@eernstg
Copy link
Member

eernstg commented Dec 28, 2023

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:

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.

In short, I still think it's misleading to add the word 'final' in those two locations.

@MaryaBelanger
Copy link
Contributor

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!

@MaryaBelanger
Copy link
Contributor

Here's what I changed:

  • Removed mentions of final added in the PR's first draft
  • Changed the statement "All uninitialized instance variables have the value null" to account for non-nullable (lines 170-172)
  • Moved the paragraph about this/initializers below the code sample on line 180, so that code sample could immediately follow the paragraph about getters and setters that it's related to.
  • Rewrote the this/initializers paragraph slightly, to emphasize that it's referring to initializing expressions (which I think is what @pdhankhar61 and I got confused about), and added a code sample to visualize the paragraph.

@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!

Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

examples/misc/lib/language_tour/classes/point_this.dart Outdated Show resolved Hide resolved
examples/misc/lib/language_tour/classes/point_this.dart Outdated Show resolved Hide resolved
src/language/classes.md Show resolved Hide resolved
src/language/classes.md Outdated Show resolved Hide resolved
src/language/classes.md Outdated Show resolved Hide resolved
@pdhankhar61
Copy link
Contributor Author

pdhankhar61 commented Dec 30, 2023 via email

@MaryaBelanger
Copy link
Contributor

/gcbrun

@dart-github-bot
Copy link
Collaborator

Visit the preview URL for this PR (updated for commit 2de12d3):

https://dart-dev--pr5310-patch-2-f15bl80n.web.app

@MaryaBelanger MaryaBelanger changed the title Update classes.md Clarify use of this in initializing instance variables Jan 3, 2024
@MaryaBelanger MaryaBelanger merged commit 78fd0c2 into dart-lang:main Jan 3, 2024
8 of 9 checks passed
@parlough
Copy link
Member

parlough commented Jan 3, 2024

Thank you @pdhankhar61 and @MaryaBelanger for working on this!! I'm excited for the improvements to have landed :)

@parlough parlough removed the review.copy Awaiting Copy Review label Jan 3, 2024
@pdhankhar61
Copy link
Contributor Author

pdhankhar61 commented Jan 4, 2024 via email

atsansone pushed a commit to atsansone/site-www that referenced this pull request Jan 26, 2024
)

Co-authored-by: Marya <[email protected]>
Co-authored-by: Marya Belanger <[email protected]>
Co-authored-by: Erik Ernst <[email protected]>
Co-authored-by: Eric Windmill <[email protected]>
Co-authored-by: Eric Windmill <[email protected]>
Co-authored-by: Parker Lougheed <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants