Dart VM does not allow a mixin of a class with itself

[iarkh]

[Edit Aug 1st 2018, eernstg: The thread below contains a discussion about the associated language design. It was concluded, and the actual task of the issue is to implement the proposal which is described here.]

---

• Dart SDK Version: 2.0.0-dev.64.1
• OS: Windows 10

Dartanalyzer passes the following example without any error or warning:

class M {}
class O extends M with M {}
main() {}

At the same time, dart throws error here. I believe analyzer should throw compile error too here. Please note that "class M extends Object with M" fails both with analyzer and dart. Sample output is:

D:\DART\sdk\tests\co19\src\LanguageFeatures\Instantiate-to-bound\inst_to_bounds>dart --version
Dart VM version: 2.0.0-dev.64.1 (Thu Jun 21 08:47:55 2018 +0200) on "windows_x64"

D:\DART\sdk\tests\co19\src\LanguageFeatures\Instantiate-to-bound\inst_to_bounds>dartanalyzer test.dart
Analyzing test.dart...
No issues found!

D:\DART\sdk\tests\co19\src\LanguageFeatures\Instantiate-to-bound\inst_to_bounds>dartanalyzer --preview-dart-2 test.dart
Analyzing test.dart...
No issues found!

D:\DART\sdk\tests\co19\src\LanguageFeatures\Instantiate-to-bound\inst_to_bounds>dart test.dart
'file:///D:/DART/sdk/tests/co19/src/LanguageFeatures/Instantiate-to-bound/inst_to_bounds/test.dart': error: line 2 pos 9: super type 'M' may not be listed in implements clause of class '_O&M&M@17011501'
class O extends M with M {}
        ^

D:\DART\sdk\tests\co19\src\LanguageFeatures\Instantiate-to-bound\inst_to_bounds>

[lrhn]

This hits the rule that a class may not implement its superclass. There is no technical reason for that, it's just telling people to avoid redundancy. It would make more sense if we disallowed all duplicate implementations (like extends List implements Iterable or implements List, Iterable.

The super-class of O is M with M which is really M implements M { /* members of M */ }, and the VM sees this as a class implementing its superclass.

I don't think we should see it that way. If the code that the user wrote is not explicitly disallowed, it is allowed. The fact that it's rewritten as something else internally should never introduce new errors. We are effectively giving an error message for code the user didn't write (there is no class _O&M&M in the source). That's bad usability.

I would say that the code should be valid, and this is really a VM bug. Leaf, WDYT?

[iarkh]

You are right, M with M can be treated as M implements M. My thought was that dart and dartanalyer should behave in the similar manner, i.e. either both don't throw error or both do throw it. Now I see that analyzer passes whereas dart throw exception.

[lrhn]

The tools should agree. The spec is a little vague on this, but it's fairly consistent in places where it says that code A is equivalent to code B in that it only applies to run-time semantics, not to static warnings (unless it explicitly says so). So, it's a warning (now error) if a class declares that it implements its super-class, but that does not happen here. The fact that code is equivalent to some code that no-one wrote, and that code would have had a problem if it was written, should not matter.

[leafpetersen]

The super-class of O is M with M which is really M implements M { / members of M / }

Not sure if it matters, but this isn't really true for super-mixins, since there are two copies of the state of M implied, and they are accessible via super calls.

[eernstg]

I'm not thrilled about mixing in multiple copies of the same mixin in the same class in the first place, but if we really want to support that kind of monster then I can't see why we would prevent the expression of that desire in a single location (class M2 = M with M;), considering how semantically similar that would be to any other construct which puts two copies of M into the same class indirectly.

I don't think we can eliminate duplication among superinterfaces in general; e.g., class C implements D {} will have two occurrences of Object in the supertype graph of the implicit superclass and that of D. ;-)

[lrhn]

It is indeed not true that M with M can be treated as M implements M.

What I wrote was that it was equivalent to M implements M { /* members of M */}, which does have two versions of the state of M (one in the super-class and one in the "members of M"). That's not the issue here. The issue is that:

class O = A with B;

is equivalent to (in some regards)

class O extends A implements B { /* members of B copied over */ }

When A and B is the same class, the VM looks at this expansion and says that it's a class which also implements its superclass's interface (which we disallow because it's useless and maybe symptomatic of a mistake).

I am saying that this is wrong behavior on the VM's behalf. It's reporting an error about doing something that the author of the code never wrote, because of an internal expansion of the actual code into something that violates a rule we have for code. Specified errors only apply to the actual input program, unless the spec explicitly says otherwise.

[eernstg]

I focused on having two copies of the instance members of M because I find that more important (e.g., we may have a foo() => something + super.foo(); where it matters if we do it multiple times), but I acknowledge that there is also the issue about "don't implement the interface of your superclass because that's silly and you probably meant something else". ;-)

If we agree that the VM should stop making this an error (because it isn't in the developer's code), we could also consider to eliminate the "don't implement your superclass" rule entirely and have a lint for that instead. That lint could then check for other situations (like the case where we have implements B for some B which is further up in the superclass chain, or implements B2 where the superclass is B and B2 is already implemented by B).

However, I don't think it's possible (or at least realistic) to strictly avoid those duplicates, and I already mentioned the example where both the superclass and each of the classes that it implements will have Object as a shared superinterface.

[eernstg]

I believe the current situation is as follows:

• If we decide that it is not allowed to apply the same mixin twice in the construction of a class (if generic, that holds when the same type arguments are provided, but it's already an error to provide different type arguments), then obviously class O extends M with M {} is an error. It seems unlikely that we will decide on doing that (it's obviously breaking to introduce such a constraint, and it does have a well-defined semantics, ugly though it may be, when we have multiple copies of the same mixin).

• Otherwise, we may make class O extends M with M {} an error because it is specified to have the effect that O implements M because of with M, and we have the rule that it is an error for a class to have its superclass in implements. That's not particularly convincing because the spec language explicitly requires the superclass to occur in an implements clause, and the reason for this restriction is also less than compelling (it's just "pointless"). It's not obvious to me that we should make it an error on that basis.

Hence, we don't have a very good reason to make it an error, and I'd suggest that we allow it. We could then have a lint to point out that it's likely to be a bad idea (and the same approach could be used for the "pointless" superclass-in-implements case which is currently an error).

@leafpetersen, @lrhn, are you happy about allowing it, such that we can close this issue and get it implemented, or just treat it as a known issue? Given that it's a non-breaking change to allow it, we should be able to introduce it later if it is impossible to get it implemented right now.

[leafpetersen]

Fine with me.

[eernstg]

Given that @leafpetersen has agreed here, and @lrhn has marked this as a vm bug, I conclude that the whole language team confirmed my proposal:

It is allowed to use class O extends M with M {} as written in the initial entry in this issue, and the vm should be adjusted to not fire the super type .. may not be listed in implements clause error when that element in the implements clause was introduced by a mixin application.

Hence, I'm removing the language team members from the list of assignees.

@a-siva, may I ask for a bit of help from you to make sure that this issue gets assigned to a relevant person from the vm team, or whatever is needed in order to bring it back into the regular workflow?

I'll edit the initial entry in this issue to add a reference to this comment, thus making it easier to see what the actual task is.

[kmillikin]

The VM should be able to recognize this case. There is a flag that marks the mixin implementation class as one that was introduced by the front end.

We should strongly consider to handle the error case M implements M in the front end and let the VM assume that the classes are well-formed in the absence of front-end errors. We could let the VM remove its checking code (except perhaps in its C API).