language icon indicating copy to clipboard operation
language copied to clipboard

Specify private override errors

Open eernstg opened this issue 2 years ago • 4 comments

Cf. https://github.com/dart-lang/language/issues/2263#issuecomment-1145106457, this PR eliminates the notion of a 'combined interface' from the language specification and clarifies the specification of how to check override errors. The main point in doing that is that it specifies how incorrect overrides are detected and reported in the case where a class or a similar declaration in a library L gives rise to an error involving private members that aren't accessible to L.

The discussion about this change gave rise to the following summary of the rules.

I've added some remarks inline to indicate that we do in fact have these properties.

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

We already specify this requirement for methods, abstract as well as concrete in line 2724.

This PR moves that rule from the section 'Instance Methods' to the section 'Classes', because it is relevant to getters and setters as well (the rule was worded correctly, but it was basically misplaced).

  • The declared signature of m from C becomes the signature of m in the interface of C

This is already specified in line 4917.

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).

This was not stated unambiguously. This PR changes \ref{interfaceInheritanceAndOverriding} such that it is explicitly defined to be a 'member signature conflict' if an interface brings together two member signatures such that no combined member signature can be computed, and the section about classes says that it is a compile-time error if the interface of the class has any of these member signature conflicts.

This covers mixins as well, because of the phrase 'It is a compile-time error for the mixin declaration if the $M_S$ class declaration would cause a compile-time error' in the section about mixin declarations.

  • The combined member signature of m becomes the signature of m in the interface of C.

This is defined as the combined member signature of a name with respect to a library, and it is defined as mentioned here in \ref{interfaceInheritanceAndOverriding}.

  • 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.

This is already covered by the section 'Fully Implementing an Interface'.

  • 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.

This is already covered here.

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.

eernstg avatar Jun 07 '22 17:06 eernstg

Visit the preview URL for this PR (updated for commit 2d1bbeb):

https://dart-specification--pr2283-specify-private-over-30v5vvvy.web.app

(expires Mon, 18 Jul 2022 17:59:29 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 6941ecd630c4f067ff3d02708a45ae0f0a42b88a

github-actions[bot] avatar Jun 07 '22 17:06 github-actions[bot]

One more thing needs to be handled: We currently rely on mixins to give rise to an implicitly induced class. This class must contain a set of members which is based on the instance member declarations in the given mixin M (or, if it's a class M which is applied as a mixin: the class M).

This is only possible when we rely on the magic ability of a language processing tool to create declarations whose name is a private name in another library. So we'd need to explain how this "magic" feature can exist, except that it is only available to the compiler (in general: some feature is not available to developers who write software, only to "the system" in some sense). We can't explain the semantics of mixin application without mentioning this feature.

eernstg avatar Jun 14 '22 15:06 eernstg

That comes back to our definition of mixin application - are we copying, or allowing the same member to occur in more than one library. (And then there's the covariant modifications happening when moving a non-covariant method implementation to a place where it implements a covariant interface method. I'm tending toward saying that the same class contains both the member declared by the mixin declaration and an override of that which adds covariant and forwards to the other method. That's two concrete methods introduced by the same class.)

lrhn avatar Jun 14 '22 15:06 lrhn

July 11th: Added some extra \commentary{..} where the text seemed to need it.

eernstg avatar Jul 11 '22 17:07 eernstg