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

Specify the behavior for an augmented constructor with no body #3555

Open
jakemac53 opened this issue Jan 11, 2024 · 7 comments · May be fixed by #3737
Open

Specify the behavior for an augmented constructor with no body #3555

jakemac53 opened this issue Jan 11, 2024 · 7 comments · May be fixed by #3737

Comments

@jakemac53
Copy link
Contributor

Specifically, given this original class:

class A {
  final int x;
  
  A() {
    print('hello!');
  }
}

And the augmentation:

augment class A {
  A() : x = 1;
}

What is the resulting constructor body? #3554 is also relevant here.

It could be any of these, or possibly others:

  • A normal implicit empty body introduced by the augmentation because of the trailing ;.
    • Probably most consistent with existing constructors, but not what the user wants.
    • This makes it difficult as well to augment only the initializers (or maybe impossible).
  • Just the original body, omitting a body in the augmentation does not augment the original.
    • This is likely what the user intended
  • An implicit "forwarding" constructor, but what would this actually mean???
@lrhn
Copy link
Member

lrhn commented Jan 12, 2024

I wouldn't say that ; introduces an empty body.

For constant generative constructors, you have to write ; because you are not allowed to have a body.
It would be inconsistent to say that ; still introduces an empty body.
So let's say that non-redirecting generative constructors can have no body, and ; is how you write it.

Still, constructor bodies are not like normal function bodies, they're more distanced from the parameters,
they're implicitly invoked in reverse super-constructor order, etc.

Which suggests that, fx, calling augmented() in the body is the wrong place to do it, and the correct place is in super-constructor position:

class A {
  final int x, y;
  A(int y) : y = log("A.y", y), super() { print("A.body"); }
}
augment class A {
  A() : x = log("AugA.x", 1), augmented(log("AugA.super", 2) { print("AugA.body"); }
}
void main() {
  new A();
}
T log<T>(String text, T value) { 
  print(text);
  return value;
}

would print: "AugA.x", "AugA.super", "A.y", "A.body", and "AugA.body".

A non-redirecting generative constructor augmentation can

  • Have an augmented() constructor invocation which chains to the augmented constructor, like you would a super-constructor.
  • Also introduce a super() constructor invocation if the augmented constructor doesn't have one already.

Which means you can write augment A { A() : this.x = 1, augment(2), super(3) {body} which will chain to the augmented constructor, and then, when it is done, continue with the superclass super(3) constructor.

Or syntactically:

class A {
  A(params1) : init1 {body1}  // no super
}
augment class A {
  A(params2) : init2, augment(args1), super(args2) {body2}
}

would work (kind-of) like rewritten to:

class A {
  A(params2) : this._$(params2, args1);
  A(params2, params1) : init2, init1, super(args2) { body1; body2; }
}

or as an example:

class A extends S {
  final int x, y, z, w;
  A(this.z, int w) : this.w = w { print("A1"); }
}
augment class A {
  A(this.x, int y) : this.y = y, augment(x + 1, y + 1), super.name(42) { print("A2"); }
}

would be (kind-of) equivalent to:

class A extends S {
  final int x, y, z, w;
  A(int x, int y) : this._$(x, y, x+1, y + 1);
  A(this.x, int y, this.z, int w): this.y = y, this.w = w, super.name(42) { print("A1"); print("A2"); }
}

@jakemac53
Copy link
Contributor Author

jakemac53 commented Jan 12, 2024

I see where you are going with that for sure.

I worry a bit that the semantics around super calls are a bit weird and nuanced. The macro API today also does not allow you to see initializer expressions , so a macro can't tell if there is already a call to a super constructor. While not directly related to these proposed semantics, it probably should still have some influence on the design.

Also, we don't generally allow changing parameters of functions through augmentation. I think you could argue that this isn't happening in this case, but especially with the initializing formals it does feel like you are altering it to me, because while the type signature might not be changing, the meaning definitely is. If I see A(this.x) I expect that the constructor will initialize the field x with exactly the value I passed. But this kind of augmentation allows you to actually alter that value, before assigning it, in a way that is likely unexpected. It could be argued that this isn't much different than any other function augmentation altering the value of parameters before passing them to the original, but I think it is.

@jakemac53
Copy link
Contributor Author

In fact, in your example, the constructor a user sees looks like it is initializing z with the first parameter, but actually it is intializing x.

@lrhn
Copy link
Member

lrhn commented Jan 12, 2024

True. In my example, the new constructor is more like a replacements, that calls the original somehow.
Not consistent with how other augementations work.

Augmentations have ... two? three? ... modes of operation:

  • Add new, previosuly missing things to existing, possibly incomplete, declarations. Like adding types, modifiers, etc.
  • Overlay (augment) functionality on top, which can call back into the original. Must still be "valid override". Can use augmented to get to prior version.
  • Replace something existing. (Not sure if that's really true, but an overlay that doesn't call augmented() might count).

If a constructor is declared as C();, then almost everything is missing, so there should be amble room to add things.

It could make sense to be allowed to add at least one of:

  • initializer list entries (individual assignments, asserts, and possibly super-constructor invocation)
  • generative forwarder (this.name(args))
  • new optional parameters
  • default values to existing optional parameters?
  • body
  • const
  • external
  • (add types to existing parameters? Seems breaking.)

(The forwarder is mutually exclusive with initializer list and body, and external is incompatible with everything except const).

These are all incremental and backwards compatible. Anything you think you can do based on the original declaration, should still work.

It would be nice to be able to add factory, but that is an incompatible change compared to the original, because now it can no longer be used as a super-constructor in subclasses, which the original declaration looks like it can.

It should be possible to keep adding things that are not incompatible with what already exists. More initialize list entries, more optional parameters. Add things that you can have more than one of.

But not more bodies, which is where the "modifying" comes in.
That could include:

  • Modifying an existing parameter to be a this. or super. parameter. (Tricky, because it makes the parameter variable final, and changes its scope to not include the body, but probably very useful when people just write a signature, and expect to get an implementation.)
  • Augment the body, allowing a body to call augmented() to run the body of the super-constructor. It's not possible to pass arguments - not unless we get really clever with how those arguments are getting bound to parameters in the augmentee body, without going through the actual parameter list. It's probably better to just inherit the bindings for the actual parameters.
  • Augment any initializer list expression. Say x = e; augmented to x = (... augmented ...) which refers to the prior expression.

All of these can work (mostly) without breaking what you can already do with the previous version of the constructor.
It treats the body almost as a separate __init__() { ... } function that you can augment.
(The real devil in the details will be how parameter variables are bound.)

@jakemac53
Copy link
Contributor Author

jakemac53 commented Jan 12, 2024

These are all incremental and backwards compatible. Anything you think you can do based on the original declaration, should still work.

Yes, but also I think it is good if all parameters you can pass are clearly documented by the original signature. This is partly why we don't allow adding optional parameters to functions generally (although that is also not fully backwards compatible due to overrides for methods).

Even though it would be fully safe (I think) in this case, outside of some very esoteric cases involving function type inference, I don't like the idea of adding parameters generally. It is just too surprising.

I feel similarly about const but not as strongly. But I don't for instance like the idea of a macro that adds const wherever it determines it safely can. This more likely than not would just lead to accidental breaking changes as constructors magically become non-const when adding new fields, and users don't realize it. And this is the only use case I can dream up for the feature.

@lrhn
Copy link
Member

lrhn commented Jan 14, 2024

The way augmentations are currently specified, I agree that anything that changes what you can and cannot do with a declaration is not a good fit. So no adding const or factory if it's not already there. Add a fresh constructor if you need one. (And don't be surprised if the class is incompatible with a const constructor.)

That leaves:

  • Adding initializer list entries. That's safe, and probably needed if someone is also adding a new field that needs initializing.
  • Adding a missing super-constructor call. Not changing an existing one.
  • Adding a missing body.
  • And wrapping an existing body. If we have that, then a missing body (;) can be treated like an empty body ({}), that does nothing if called (or not called).

Calling the existing body from a wrapper should probably be augmented() with no arguments. The scope of the super-body is the same parameters as the augmenting body, with some names removed if necessary. (Can't add parameters, can't currently introduce variables in initializer list, so no need to remove anything.)

I'd also want to add default values of optional parameters that don't have any, or maybe even change existing ones, since the new body might want different defaults. (But it's probably better to just use nullable parameters, and introduce the default value yourself, then that's moot.)

I do think that an initial declaration of C(); should allow being specialized into either a redirecting constructor or an initializing constructor, depending on what is added, because otherwise there is no syntax for a partially defined constructor that is filled in as a forwarding constructor by the augmentation. If nothing is added, it'll become a initializing constructor at the end (implicitly adding : super();).
Or it can be made into external C(); if augmentations can add external (which it should be able to, if the augmentation itself is external. so you can do @JS("Math.imul") augment external int imul(int x, int y); to augment a declaration of int imul(int x, int y);. That's a valid way to add an implementation.).

And factory C(); can similarly be allowed (temporarily) and specialized into either a redirecting or computing factory constructor, and it's a compile-time error if nothing has been added at the end.

@jakemac53
Copy link
Contributor Author

  • And wrapping an existing body. If we have that, then a missing body (;) can be treated like an empty body ({}), that does nothing if called (or not called).

My main concern is that if there is an existing body, filling in {} for the ; case will effectively just delete the existing body, which is probably never the intention (why write the original then?). So I think I would want it to leave the original in place, or introduce a body like { augmented(); }.

Calling the existing body from a wrapper should probably be augmented() with no arguments. The scope of the super-body is the same parameters as the augmenting body, with some names removed if necessary. (Can't add parameters, can't currently introduce variables in initializer list, so no need to remove anything.)

Yeah, I think this is the only real sensible option for augmenting the constructor body. It feels a bit weird, but makes sense.

@jakemac53 jakemac53 linked a pull request Apr 29, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants