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

augmented() can't occur in an initializer list of an augmenting constructor declaration, right? #4039

Open
eernstg opened this issue Aug 15, 2024 · 4 comments
Labels
augmentation-libraries question Further information is requested

Comments

@eernstg
Copy link
Member

eernstg commented Aug 15, 2024

Surely it would violate some invariants that we can otherwise rely on if augmented() is allowed to be executed in the initializer list of a non-redirecting, or in the redirection of a redirecting generative constructor. (In short: general code gets to see an object which is not initialized.)

However, I can't find an explicit indication in the feature specification that there must be such an error. Here is an example. Currently (ac9b6d1672255d2233ed612eb9c94921dca3cd3a), the analyzer does not report any errors.

class A {
  dynamic x;
  A() {
    print('Running introductory!');
  }
  augment A(): x = augmented();
}

class B {
  B(Object? o);
  B.named(): this(42);
  augment B.named(): this(augmented());
}

I think we must specify a compile-time error for both of these situations, because the initializer list / redirection does not have access to this, but the body of an augmented generative constructor can access this.

Of course, we could also allow both of them, and consider augmented to be a regular identifier in both situations, potentially resolving to a static, top-level, or imported declaration. We'd then have an error if augmented() in that context resolves to an instance member of the enclosing class/enum/extension-type, because the initializer list / redirection does not have access to this.

@dart-lang/language-team, WDYT?

@eernstg eernstg added question Further information is requested augmentation-libraries labels Aug 15, 2024
@eernstg eernstg changed the title augmented() can't occur in an initializer list? augmented() can't occur in an initializer list of an augmenting constructor declaration? Aug 15, 2024
@eernstg eernstg changed the title augmented() can't occur in an initializer list of an augmenting constructor declaration? augmented() can't occur in an initializer list of an augmenting constructor declaration, right? Aug 15, 2024
@jakemac53
Copy link
Contributor

jakemac53 commented Aug 15, 2024

It isn't explicitly specified as an error, but it is also never specified as having any meaning, which should mean it resolves to a regular identifier. We only define a meaning for augmented within the constructor body.

It certainly wouldn't hurt to make that explicit, but I am not sure where we draw the line there. Do we need to define for every single context in which an identifier can occur, what augmented means? Or can we make a blanket statement that if it is not explicitly defined, it is just a regular identifier?

@lrhn
Copy link
Member

lrhn commented Aug 15, 2024

I would disallow it or give it a meaning, because there is a reasonable meaning: an initializer list entry could augment an existing initializer list entry for the same variable and refer to the existing initializer.
If we don't want that today, we should disallow using augmented in initializer list expressions entirely, to keep the option open.
(Definitely not allowed in initializer list asset expressions, I don't want the same identifier to mean different things at different places in the same scope.)

We do need to define the meaning for each code context that can occur inside an augmenting member declaration what augmented refers to, if anything.
Initializer list entries contain code, not just declaration. They're theoretically augmentable.

@eernstg
Copy link
Member Author

eernstg commented Aug 16, 2024

So we could have this:

// `augmented` in an initializer list is is a regular identifier.

int augmented() => 1;

class A {
  int i; // Initial value is 1.
  A();
  augment A(): i = augmented();
}

Or this:

// `augmented` in an initializer list expression refers to the augmented
// expression that was used to initialize the same instance variable.

class A {
  int i; // Initial value is 4.
  A(): i = 2;
  augment A(): i = augmented * 2;
}

Or this:

// `augmented` stands for zero or more initializer list entries.
// Assume that `value` is 3 just before the constructor runs.

var value = 3; // After the constructor execution, `value` is 6.

class A {
  int i; // Initial value is 5.
  int j; // Initial value is 3.
  A(): i = value++;
  augment A(): j = value++, assert(++value > 0), augmented;
}

Or various subsets of the above.

Similar examples can be created for the redirection of a redirecting generative constructor (this(...augmented...)).

In addition to these positive properties we'd also have some negative ones: rules about locations in initializer lists or redirections where augmented() or augmented is an error.

I agree with @lrhn that there is a danger associated with the choice that augmented is just a regular identifier when it occurs in an initializer list: It turns the other semantics into breaking changes, should we want any of them in the future. Moreover, it is arguably a particularly surprising/error-prone semantics to let augmented work as a regular identifier in the initializer list if it is used to invoke the augmented constructor body in the body, which could be the very next line of the constructor declaration.

With this in mind, we still have the option to say that augmented (with or without ()) in an initializer list or in a generative redirection is a compile-time error, and then add support for a positive semantics at some point in the future. I'd recommend that we do this.

@jakemac53
Copy link
Contributor

With this in mind, we still have the option to say that augmented (with or without ()) in an initializer list or in a generative redirection is a compile-time error, and then add support for a positive semantics at some point in the future. I'd recommend that we do this.

SGTM, lets just do that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
augmentation-libraries question Further information is requested
Projects
Development

No branches or pull requests

3 participants