dart-lang / language

Design of the Dart language
Other
2.67k stars 205 forks source link

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

Open eernstg opened 3 months ago

eernstg commented 3 months ago

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?

jakemac53 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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