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

Revisit unimplemented factory constructors and default values #4172

Open
eernstg opened this issue Nov 25, 2024 · 6 comments
Open

Revisit unimplemented factory constructors and default values #4172

eernstg opened this issue Nov 25, 2024 · 6 comments
Labels
augmentation-libraries question Further information is requested

Comments

@eernstg
Copy link
Member

eernstg commented Nov 25, 2024

We need to introduce an extra rule about factory constructors that are subject to augmentation: The can omit default values, even for

With augmentations, a factory constructor can be unimplemented. It is possible to make a late choice about the nature of this constructor (it could be redirecting or it could be non-redirecting):

class A {
  A();
  factory A.foo(); // Unimplemented factory constructor.
}

augment class A {
  augment factory A.foo() = A;
}

class B {
  B();
  factory B.foo(); // Unimplemented factory constructor.
}

augment class B {
  augment factory B.foo() {
    return B();
  }
}

We may need to introduce an extra rule about these unimplemented factory constructors which says that it is allowed for an optional parameter to omit the default value clause, even in the case where the parameter type is potentially non-nullable.

For example:

class A {
  A([int i = 0]);
  factory A.foo([int i]); // Currently an error; this issue proposes to allow it.
}

augment class A {
  augment factory A.foo([int i]) = A; // Default value obtained from redirectee.
}

The general rules about augmentations state that no default value can be specified by an augmentation, it must always be specified by the introductory declaration (if there is to be a default value for that parameter at all).

This implies that the declaration above would not allow A.foo to be augmented with a class body (such that it is a non-redirecting factory constructor): The parameter type cannot be modified by an augmentation, and an augmentation cannot provide a default value. In other words, the fact that there is no default value has eliminated the expressive power to make the decision about the factory being redirecting or non-redirecting, it's always forced to be redirecting.

We could allow the unimplemented factory constructor to include the default value and then ignore it (because it would be an error to have it) when the constructor is augmented to be redirecting:

class A {
  A([int i = 0]);
  factory A.foo([int i = 1]); // Currently allowed.
}

augment class A {
  augment factory A.foo([int i]) = A; // Currently an error; this issue suggests we might allow it.
}

class B {
  B();
  factory B.foo([int i]); // Proposed: Allow this.
}

augment class B {
  augment factory B.foo([int i = 3]) { // We could allow this.
    return B();
  }
}

If we do not allow this then the choice to have a default value will force the constructor to be non-redirecting.

In other words, the ability to "decide late" whether a given factory is redirecting or not will disappear as soon as there is one or more optional parameters whose type is potentially non-nullable. Of course, we can then make the declarations inconsistent: If we have two such parameters and provide a default value for exactly one of them then no augmenting declarations can be written, because they will give rise to a compile-time error no matter what.

@dart-lang/language-team, WDYT? Are you willing to allow a default value to (1) occur in an augmenting declaration, and/or (2) be declared in the introductory declaration, but then erased in the final definition because tit would be an error?

@jakemac53
Copy link
Contributor

jakemac53 commented Nov 25, 2024

My opinion is to not change anything here other than possibly augmenting the definition of "potentially redirecting" to indicate that the presence (or absence) of a default value on an optional, non-nullable parameter will determine the type of the constructor (as in redirecting or not).

It isn't intended to be a feature that an augmentation can alter this property of the constructor, this "potentially redirecting" definition is only there to describe how we handle the case where we can't know from the introductory declaration, not to try and add flexibility.

Edit: If we don't allow providing the default value but no body today, we should allow that.

@lrhn
Copy link
Member

lrhn commented Nov 26, 2024

I do want to introduce flexibility.

If a user writes a constructor signature, and as a macro to it, I do want to give the macro the flexibility to choose the implementation.

That said, any number of details can lock the declaration onto being redirecting or not:

  • const implies factory (for now, I have an issue asking for const non-redirecting factory constructors.)
  • a default value implies non-redirecting.
  • a redirection or body is obvious.

It really is just the plain signature that is ambiguous.

@eernstg
Copy link
Member Author

eernstg commented Nov 26, 2024

One approach we could use would be to say that the default value can be specified in an otherwise pluripotent (sounds better than 'ambiguous' ;-) constructor declaration, and if it is an error to specify that default value in the resulting constructor definition then it will simply be ignored. We might want to maintain that it is an error if there are two conflicting default values in the same augmentation chain, or we could even allow that to enhance the overall flexibility. Example:

class A {
  A([int i = 0]);
  factory A.foo([int i = 1]); // OK.
}

augment class A {
  augment factory A.foo([int i]) = A; // OK, and the "inherited" default value is ignored.
}

class B {
  B([int i = 0]);
  factory B.foo([int i = 1]); // OK.
}

augment class B {
  augment factory B.foo([int i]) { // OK, and the default value is "inherited".
    return B(i);
  }
}

class C {
  C([int i = 0]);
  factory C.foo([int i = 1]); // OK.
}

augment class C {
  augment factory C.foo([int i = 2]) { // Error?
    return C(i);
  }
}

Taking one step back, I think we have the following main players when it comes to constructor augmentation:

graph LR
  subgraph Factory
    direction BT
    fnr["Non-redirecting<br>factory A(int i) => B(i);"] --> fp["Pluripotent<br>factory A(int i);"]
    fr["Redirecting<br>factory A(int i) = B;"] --> fp
  end
  subgraph Generative
    direction BT
    gnr["Non-redirecting<br>A(int i) {...}"] --> gp["Pluripotent<br>A(int i);"]
    gr["Redirecting<br>A(int i) : this(i + 1);"] --> gp
  end
Loading

This is already somewhat inhomogeneous because the pluripotent generative constructor is usable as such, but the pluripotent factory is a new invention (an 'unimplemented' factory), and it is an error unless there is an augmentation which will implement it (by redirection or by a body).

The fact that some of these arrows can be blocked by parameter default values makes the situation one more bit inhomogeneous.

@eernstg
Copy link
Member Author

eernstg commented Nov 26, 2024

Perhaps we should go a step further and simply say that it is not a supported feature to change the kind of constructor using augmentation? (It's a mess!)

So we would accept an introductory declaration like A(int i); (generative, non-redirecting, can add a body and/or an initializer list) or a introductory declaration like factory A(int); (factory, non-redirecting, can add a body), but they cannot be augmented into redirecting constructors. In short, the pluripotent forms are now always non-redirecting.

If you wish to declare a redirecting constructor in an augmentation chain then it just needs to be introduced as such. If there is no A(int i); in the augmented declaration then it is not an error to declare A(int i) : this.name(i + 1); in the augmenting declaration. Similarly for the redirecting factory.

I think this implies that there would not be any situations where a default value can be declared in the introductory declaration, and it becomes an error to have it in the augmenting declaration, or vice versa.

This takes away a little bit of expressive power, but it also gives rise to a more consistent (and less confusing) model.

graph LR
  subgraph Factory
    direction BT
    fnr["Non-redirecting<br>factory A(int i) => B(i);"] --> fp["Pluripotent<br>factory A(int i);"]
    fr["Redirecting<br>factory A(int i) = B;"]
  end
  subgraph Generative
    direction BT
    gnr["Non-redirecting<br>A(int i) {...}"] --> gp["Pluripotent<br>A(int i);"]
    gr["Redirecting<br>A(int i) : this(i + 1);"]
  end
Loading

@jakemac53
Copy link
Contributor

Perhaps we should go a step further and simply say that it is not a supported feature to change the kind of constructor using augmentation? (It's a mess!)

We should in general have a principle that we don't allow you to change any which could be introspected on, because that leads to different introspection results in different phases.

And, we probably do need to give you the ability to check if a constructor is redirecting or not, because it informs how you can augment the constructor.

Therefore, I think we are left with really only two reasonable options:

  • Seal the type of constructor based on its introductory declaration.
  • Expose the "pluripotent" property of constructors which could be redirecting or generative. This would have to be updated in between macros and would introduce unfortunate ordering constraints and reduced parallelism.

@lrhn
Copy link
Member

lrhn commented Dec 3, 2024

Expose the "pluripotent" property of constructors which could be redirecting or generative.

Could be .isNonRedirecting and .isRedirecting that both return false.

(Redirecting and generative are not mutually exclusive. We have almost all eight combinations of const or not, factory or generative, and redirecting and non-redirecting.)

Perhaps we should go a step further and simply say that it is not a supported feature to change the kind of constructor using augmentation? (It's a mess!)

Then it's not possible to augment a redirecting generative constructor, because it can only be redirecting if it has written a redirection, and we don't allow changing that redirection.
It's not possible to write a plain signature and allow someone else to decide the implementation, and let them choose whether that implementation is redirecting or not.

I would like that flexibility, to be able to write a signature and let a macro worry about the implementation, without my signature being taken as an implementation in itself.

Maybe allow abstract C(); as a pluripotent generative constructor, so you have something to write (and later augmentations can then add an implementation, removing the abstract. They have to, a constructor must not be abstract in the end.)

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