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 private override error caused by a mixin application #1626

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

eernstg
Copy link
Member

@eernstg eernstg commented May 10, 2021

Proposed rules for a mixin related name clash error, cf. dart-lang/sdk#28809 and dart-lang/sdk#45959.

This PR adjusts the language specification section 'Mixin Application' such that it specifies the error which arises when an override relationship arises among two concrete declarations with a private name n in a library L because of a mixin application that occurs outside L.

// Library 'lib.dart'.
class A {
  void _foo() {}  
}

class B {
  void _foo() {}
}

mixin M {
  void _foo() {}
}

mixin MA implements A {
  void _foo() {}
}

// Library 'other.dart'.
import 'lib.dart';

class AB extends A with B {} // Error.
class AM extends A with M {} // Error.
class AMA extends A with MA {} // Error.

Note that it is an error in all cases where the mixin application outside lib.dart creates an override relationship for a member with a private name (for instance, such that A._foo is overridden by MA._foo), even in the cases where the given members are already associated with each other by declarations (e.g., MA._foo is connected to A._foo by implements A, but it is still an error to have A with MA in other.dart).

Let \DefineSymbol{L_M} be the library containing the declaration of $M$.

\LMHash{}%
Assume that $S$ has a concrete member $m_S$ which is accessible to $L_M$,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we agree on it needing to be concrete?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this PR awaits the discussion in the language team: Do we want to support these mixin applications in all the cases where it does not give rise to an override, or do we want to raise an error on all clashes (abstract or concrete, that doesn't matter)?

My preferred approach is the former, because I don't want to eliminate expressive power gratuitously.

However, we certainly need to raise an error when there is an override because of an implicitly induced implementation (say, a stub that checks the type of an actual argument and calls super.sameMember).

This means that the text in this PR needs to be updated, but it should be updated in two different ways if we choose the former respectively the latter approach.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cf. the decision at the language meeting yesterday, the PR has now been updated to make it an error when the private collision as created by a mixin application, no matter what kind of clash we have (concrete/concrete, concrete/abstract, concrete/stub, abstract/abstract, it's always an error).

@eernstg eernstg force-pushed the specify_private_mixin_clash_may21 branch from 1d31e4d to ecb4102 Compare June 10, 2021 09:57
@eernstg
Copy link
Member Author

eernstg commented Jun 10, 2021

PTAL, the spec change has been updated to reflect the decision at the language meeting yesterday: All clashes are conflicts, even abstract/abstract.

@eernstg eernstg force-pushed the specify_private_mixin_clash_may21 branch from 8791e77 to bd37ac1 Compare June 14, 2021 09:55
@eernstg
Copy link
Member Author

eernstg commented Jun 14, 2021

The commentary about the private collisions was extended to motivate the error for some abstract-on-concrete cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants