Should combined member signature computation be defined WRT the current library?

[leafpetersen]

This issue is an attempt to split out one of several overlapping issues relative to this issue.

In discussion in the last language team meeting, I believe the claim was made that the following code is, per the spec, valid code which should be accepted:

// lib.dart
abstract class A {
  void _m(String x);
  int get _x => 3;
  void _n([int x]);
}

abstract class B {
  void _m(int x);
  void _x() {}
  void _n({int x});
}

// main.dart
import "lib.dart";

abstract class C implements A, B {}

void main() {}

I believe that the reason for this is that the definition of combined member signature (section 11.2.2) is done with respect to a library L, thereby filtering out inaccessible members from consideration when requiring that there exist a member signature in one or more of the super-interfaces which is a subtype of all of the others.

@eernstg is this a correct summary of the position WRT to the current specification, or have I missed other relevant bits?

Note that none of our implementations currently accept this program: the definition of class C causes errors to be emitted for all three members.

Note that there is no concrete member which can implement _n, nor _x. For _m there is a valid signature which implements both, but we do not specify any way of computing this, and neither declaration of _m is a subtype of the other.

It seems to me that allowing this program is very contrary to the notion of combined member signature. There is no valid combined signature which can be computed. Practically speaking, this becomes an issue for the question of generating a noSuchMethod forward for these members: what is the signature (what even is the calling convention?) for the generated forwarder?

I believe that we should specify this program to be an error, and impose the combined member signature requirements to all members, accessible or not. Thoughts on this?

cc @eernstg @munificent @lrhn @jakemac53 @natebosch @chloestefantsova @johnniwinther @scheglov

[leafpetersen]

cc @stereotype441

[eernstg]

I believe we do have a specified compile-time error for the member signature clashes. We have the notion of a 'combined interface', and the combined interface of a list of interfaces includes the combined member signature for all members, for all libraries where the given member is accessible, cf. this spec location:

The \Index{combined interface} $I$ of a list of interfaces \List{I}{1}{k}
is the interface that declares the set of member signatures $M$,
where $M$ is determined as specified below.
The \Index{direct superinterfaces} of $I$ is the set \List{I}{1}{k}.

Let $M_0$ be the set of all member signatures declared by \List{I}{1}{k}.
\DefineSymbol{M} is then the smallest set satisfying the following:

\begin{itemize}
\item For each name \id{} and library $L$ such that $M_0$ contains
  a member signature named \id{} which is accessible to $L$,
  let $m$ be the combined member signature named \id{}
  from \List{I}{1}{k} with respect to $L$.
  It is a compile-time error
  if the computation of this combined member signature failed.
  Otherwise, $M$ contains $m$.
\end{itemize}

\rationale{%
Interfaces must be able to contain inaccessible member signatures,
because they may be accessible from the interfaces associated with
declarations of subtypes.%
}

So in the example here the computation of the combined interface of A, B in the static analysis of class C would give rise to a compile-time error, for each of the member names _m-in-lib.dart, _x-in-lib.dart, and _n-in-lib.dart.

However, it looks like the language specification is vague when it comes to the computation of a combined interface: I don't think it's ever specified that we must compute the combined interface for A, B when C is checked.

[leafpetersen]

However, it looks like the language specification is vague when it comes to the computation of a combined interface: I don't think it's ever specified that we must compute the combined interface for A, B when C is checked.

Yes, I cannot find any place in the spec where that definition is used. The only uses of combined with respect to an interface that I can find are in the definition, or in commentary, or as combined superinterface in the section on mixins.

[eernstg]

We wouldn't simply specify that the combined interface of all superinterfaces of every class/mixin is computed, and the compile-time error may then occur: We don't want spurious compile-time errors for a class based on public member signatures from superinterfaces that do not have a combined member interface.

abstract class A { void m(int i, double d); }
abstract class B { void m(double d, int i); }

class C implements A, B {
  void m(num n, num n) {}
}

So we could drop the notion of 'combined interface' entirely and re-word the checks on classes and mixins such that they require a combined member signature to exist for every member of a direct superinterface (public or private to any library) that is not overridden in the class itself. I think we have almost all of that already, but the explicit reference to private methods seems to be mentioned with 'combined interface' only, so that but would be reorganized.

[leafpetersen]

Ok, after discussing this further with @lrhn and @eernstg I think we're all in agreement that the correct resolution is as @eernstg says above, i.e. "require a combined member signature to exist for every member of a direct superinterface (public or private to any library) that is not overridden in the class itself". More concretely, this means that we end up with the following as the proposed semantics for the errors and warnings around a given method m with respect to a class declaration C in library L:

• Case: m is declared in C (C is abstract, or concrete, m must be accessible in L by definition)
  • We don’t require that m have a combined signature in the super-interfaces
  • We do require that m is a proper override of all signatures for m from all direct super-interfaces
  • The declared signature of m from C becomes the signature of m in the interface of C
• Case: m is not declared in C (m may be accessible in L or not)
  • We require that a combined member signature for m WRT to the direct super-interfaces of C exist, including private members (i.e. even if m is not accessible in L, we still require that a combined member signature exist for m).
  • The combined member signature of m becomes the signature of m in the interface of C.
  • If C is abstract, we are done: we do not require that C implement its interface
  • If C is concrete and m is public, we separately require that the concrete implementation of m implement its signature in the interface as defined above.
  • If C is concrete and m is private to a different library, we introduce a noSuchMethod thrower (forced by privacy) with the combined member signature for m as defined above.

These restrictions enforce the following properties:

• Every class (abstract or concrete) has a single well-defined signature for every member (accessible or not) in its interface
• Every concrete class has a single well-defined concrete implementation of every member (accessible or not) in its interface, and that implementation correctly implements the signature from the previous bullet.

[lrhn]

We do require that m is a proper override of all signatures for m from all direct super-interfaces

(Technically we require it to be a proper override of all signatures in all super-interfaces, not just direct super-interfaces.
It just happens that if there are no covariant parameters, it's sufficient to check the direct super-interfaces, since those are also guaranteed to be valid overrides of their transitive super-interfaces, and being a valid override is transitive except for covariant. I'm sure we can make yet another kind of conflict using covariant private members.)

• If C is concrete and m is private to a different library, and C does not inherit a concrete implementation of m which implements the combined signature for m as defined above we introduce a noSuchMethod thrower (forced by privacy) with the combined member signature for m as defined above.

It's possible to inherit a correct implementation. In that case, we do not need to introduce a noSuchMethod thrower.

Otherwise I agree.

(I then want to introduce lints that warn you if your private members risk causing conflicts for people in other libraries, because those people usually can't fix the conflicts.)

[eernstg]

@lrhn wrote:

Technically we require it to be a proper override of all signatures in all super-interfaces, not just direct super-interfaces.

That isn't quite true, we have defined the interface of a class C (of particular interest here: the ones that occur as direct superinterfaces of some other class or mixin or view) such that it contains the members that C declares as well as any others that C 'has' (because some direct or indirect superinterface of C declares it). Also, the individual member signatures of the interface of a class B have the modifier covariant on each parameter which is 'covariant by declaration'. In other words, the interface of a class contains the results of traversing the entire superinterface graph.

This is crucial for programs that include libraries both with and without null safety, and convenient in the simpler setting where everything has null safety.

But we do need to double check that the special rule (which is outside the definition of what it means to be a 'correct override') about the relationship between the declared type of a parameter which is covariant by declaration, and the declared types of all corresponding parameters in all superinterfaces (direct or indirect) will also be performed for members that are private to other libraries than the current one.

It's possible to inherit a correct implementation. In that case, we do not need to introduce a noSuchMethod thrower.

I believe this would be covered by 'and C does not inherit a concrete implementation of m which implements the combined signature for m as defined'.

I then want to introduce lints that warn you if your private members risk causing
conflicts for people in other libraries, because those people usually can't fix the conflicts.

+100!

[stereotype441]

@leafpetersen said:

Ok, after discussing this further with @lrhn and @eernstg I think we're all in agreement that the correct resolution is as @eernstg says above, i.e. "require a combined member signature to exist for every member of a direct superinterface (public or private to any library) that is not overridden in the class itself". More concretely, this means that we end up with the following as the proposed semantics for the errors and warnings around a given method m with respect to a class declaration C in library L:

• Case: m is declared in C (C is abstract, or concrete, m must be accessible in L by definition)

  • We don’t require that m have a combined signature in the super-interfaces
  • We do require that m is a proper override of all signatures for m from all direct super-interfaces
  • The declared signature of m from C becomes the signature of m in the interface of C

• Case: m is not declared in C (m may be accessible in L or not)

  • We require that a combined member signature for m WRT to the direct super-interfaces of C exist, including private members (i.e. even if m is not accessible in L, we still require that a combined member signature exist for m).
  • The combined member signature of m becomes the signature of m in the interface of C.
  • If C is abstract, we are done: we do not require that C implement its interface
  • If C is concrete and m is public, we separately require that the concrete implementation of m implement its signature in the interface as defined above.
  • If C is concrete and m is private to a different library, we introduce a noSuchMethod thrower (forced by privacy) with the combined member signature for m as defined above.

These restrictions enforce the following properties:

• Every class (abstract or concrete) has a single well-defined signature for every member (accessible or not) in its interface
• Every concrete class has a single well-defined concrete implementation of every member (accessible or not) in its interface, and that implementation correctly implements the signature from the previous bullet.

Should this apply to the synthetic classes that are created as part of de-sugaring with clauses? In other words, I believe this code to be illegal under Leaf's proposal:

class A {
  void f(int i) {}
}
abstract class I {
  void f(String s);
}
mixin M implements I {}
class AwithM = A with M;
class B extends AwithM {
  void f(Object o) {}
}

because in the class AwithM, the concrete member void f(int i) {} fails to implement the signature void f(String s); from I. But the class B is ok, because it overrides f with a concrete member that satisfies both signatures.

If I'm right about that, then what does that mean about this "equivalent" code, in which the class AwithM doesn't really exist, except as a synthetic artifact of the declaration of B?

class A {
  void f(int i) {}
}
abstract class I {
  void f(String s);
}
mixin M implements I {}
class B extends A with M {
  void f(Object o) {}
}

Note that currently the CFE rejects both of these examples, and the analyzer accepts both of them 😢

[lrhn]

The mixin application classes introduced by extends A with M are abstract. The desugaring should introduce abstract classes too.

[eernstg]

@stereotype441, the rules about correct overrides (and similar checks, e.g., the one about covariant parameters' types) are specified in more than one location in the language specification, and I expect that the update which is needed in order to clarify the treatment of inaccessible private overrides will mostly consist in making it explicit that correct overrides must be checked from any library, not just the current library. Other than that, Leaf's rules would be a restatement of the current rules, and I'll start a discussion about this if the rules turn out to imply further changes.

class A {
  void f(int i) {}
}

abstract class I {
  void f(String s);
}

mixin M implements I {}

class AwithM = A with M;

class B extends AwithM {
  void f(Object o) {}
}

With the current rules we should get an error at AwithM because that class is concrete, and it does not have a concrete implementation of f with a signature that is a correct override of the one in A as well as the one in M, and a concrete class "must fully implement its interface":

language/specification/dartLangSpec.tex

Line 2575 in 812f939

\subsection{Fully Implementing an Interface}

We do not specify the handling of dependent errors, which means that class B may or may not be an error (because it uses AwithM as a superinterface, and AwithM has a compile-time error, and it's even specifically an error about f).

class A {
  void f(int i) {}
}

abstract class I {
  void f(String s);
}

mixin M implements I {}

class B extends A with M {
  void f(Object o) {}
}

With this variant the implicitly induced superclass of B (denoted by the syntax A with M) is abstract, as @lrhn mentioned, so it's not a problem that there is no implementation of some of the members in its interface. We get an error anyway: Let's call that superclass S, then we have the following implicitly induced declaration:

abstract class S extends A implements M {}

Class S is a compile-time error because it has no member signature for the member named f: At least one direct superinterface has a member named f, and S doesn't declare a member with that name, and the computation of the combined member interface for f in the direct superinterfaces fails.

We could have a special exception saying that these implicitly induced classes don't have to have a complete interface, so they could have certain members whose signature is ambiguous as long as they're part of a declaration of a class (like B) where each of those ambiguous signatures is resolved.

However, it could just as well be argued that this kind of ambiguity in a mixin application used as a superclass should be detected and reported, because it's likely to be a mistake.

The CFE already reports an error (whose explanatory text is compatible with the perspective that the error is caused by an ambiguity in member signatures). I'd recommend that we rely on this behavior (which makes it a change that isn't breaking in practice), and change the analyzer to report the error as well.

[lrhn]

We could make class A extends B with M1, M2 { ... } not require B with M1, M2 to have a consistent interface for each of B with M1 and B with M1, M2, as long as combining them with the body { ... } makes the signatures unambiguous, but that would still mean finding the combined interface for specific members if someone does super.foo() in the body.

Not sure it buys us much.

[eernstg]

Cf. #2283.