dart-lang / language

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

Revisit unimplemented factory constructors and default values #4172

Open eernstg opened 1 week ago

eernstg commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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:

It really is just the plain signature that is ambiguous.

eernstg commented 6 days ago

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

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 commented 6 days ago

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