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

dart2js throws Internal Error when a class implements an interface with two different type parameters #14729

Closed
justinfagnani opened this issue Nov 1, 2013 · 13 comments
Labels
closed-obsolete Closed as the reported issue is no longer relevant P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) web-dart2js

Comments

@justinfagnani
Copy link
Contributor

The error when compiling one of the package:js samples:

/Users/justinfagnani/Documents/Projects/js-interop/example/google-chart/packages/js/js.dart:243:1: Internal Error: Inheritance of the same class with different type arguments is not supported: Both Serializable<FunctionProxy> and Serializable<Proxy> are supertypes of FunctionProxy.
class FunctionProxy extends Proxy
^^^^^
/

The basic construction that package:js uses is:

class A implements X<A> {}
class B extends A implements X<B> {}

@justinfagnani
Copy link
Contributor Author

cc @gbracha.

@gbracha
Copy link
Contributor

gbracha commented Nov 1, 2013

FWIW, the spec does not have such a restriction. I recall some other language whose generic scheme had such rules. I can't quite remember its name - something like Fava maybe?

@kasperl
Copy link

kasperl commented Nov 2, 2013

Unfortunately this is an intentional restriction in dart2js for now. Implementing the same interface with different type arguments has never really worked, so we felt very uncomfortable having people depend on the broken behavior.

For now, I think you should look into fixing it in package:js. I'll take a look at how to do that on Monday.


cc @johnniwinther.

@kasperl
Copy link

kasperl commented Nov 4, 2013

Added this to the Later milestone.
Removed Priority-High label.
Added Priority-Medium label.

@kasperl
Copy link

kasperl commented Jul 10, 2014

Removed this from the Later milestone.
Added Oldschool-Milestone-Later label.

@kasperl
Copy link

kasperl commented Aug 4, 2014

Removed Oldschool-Milestone-Later label.

@greglittlefield-wf
Copy link
Contributor

greglittlefield-wf commented Jan 21, 2016

Any updates on this? I'm running into a similar issue, except with extends/implements flip-flopped:

class X<T> {}

class A {}
class B extends A {}

class Foo extends X<A> {}
class Bar extends X<B> implements Foo {}

[error on line 8] Dart2js does not currently support inheritance of the same class with different type arguments: Both X<B> and X<A> are supertypes of Bar.

I can see why this restriction is in place for most cases, but it seems like it should be fine when one generic parameter is a subtype of the other.


Update: this specific example has the following analysis error in Dart 2.0.0-dev.63.0:

The class 'Bar' cannot implement both 'X<B>' and 'X<A>' because the type arguments are different.

@kevmoo kevmoo added P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) and removed triaged labels Feb 29, 2016
@ajrcarey
Copy link

ajrcarey commented Mar 14, 2017

Any updates on this problem? Dart2JS will not compile code that passes strong-mode analysis at present.

@sigmundch
Copy link
Member

Thanks for the ping - there hasn't been any changes so far regarding this in dart2js. This is unfortunately a very hard thing to change: dart2js tracks runtime types in a way that associates one type instantiation per value, adding more (e.g. to track both X<A> and X<B>) will require changing the entire mechanism used today to reify runtime types.

It's quite possible that the resolution will be to not change the current behavior depending on what the language designers decide. I personally believe this is a case where the language should be stricter, especially in strong-mode. @vsmenon @leafpetersen - what do you think?

For context, right now strong-mode only complains when there are contradicting members, for example, if you change the example above to:

class X<T> {
  foo(T x) {}
}

then strong-mode complains. Because foo(B) is an invalid override for foo(A).

Funny though that if I write a program like:

main() {
  var x = new Bar();
  print(x as X<A>);
  print(x as X<B>);
}

I see hints saying that the casts are unnecessary, but I'm guessing one of them will fail at runtime (at least in DDC), correct?

@ajrcarey
Copy link

ajrcarey commented Mar 15, 2017

Thanks for updating this. I am about as far away from being a language designer as it's possible to be - I'm just a lowly programmer. So I will not pretend to understand the ins and outs of the design constraints. But as a user of the tools, the thing that is most noticeable to me is that the tools are not in agreement about what the language can do. The analyzer accepts code like that which @greglittlefield-wf provided on Jan 21; dart2js doesn't. The main problem, from a utilisation perspective, is that the tools don't agree on what the language accepts. So you end up in a situation where you write code, the analyzer tells you it's ok, but then things blow up when you try and build it. This is a problem.

Personally, I'd actually prefer it if the analyzer told me that I'm writing code that dart2js isn't going to accept. That way I immediately know I have a problem - I don't get a nasty surprise when I get to the build stage. This would be greatly preferable. I don't mind the constraint that dart2js applies in this situation - in fact, having had some time to give it more thought, I can see why the situation is sufficiently ambiguous that dart2js flags it as an error. So I'd prefer that the analyzer told me the code isn't acceptable to dart2js, rather than telling me it's perfectly fine and then having things blow up later on.

(While I haven't checked this specific situation, I'm going to go out on a limb and guess that Dartium will also accept the code and run it without complaint - thus exacerbating the situation of the tools not being in agreement. The dev toolchain accepts a certain code construct, and the test toolchain does as well - only when you try to build for production do you find out that "no, actually, you can't write that, sorry". This is far, far too late in the development process to be finding out about build-breaking problems.)

I know that not everyone is targeting dart2js, but for those of us that are, perhaps some opt-in analysis options to enable IDE support for dart2js-specific language constraints? Personally, I'd be happy with that because it would give me greater confidence that the code I'm writing is going to deploy without problems. At the moment, problems like this one have resulted in a situation where I'm really not all that confident that I can actually build and deploy my code even when my other tools are telling me that my code is correct. That dents my confidence in the toolset overall which is such a shame when the language is so well designed and, individually, the tools are so strong.

@sigmundch
Copy link
Member

@ajrcarey - I couldn't agree more. You might have seen in the other bug that we are currently working on sharing more code between these tools so that they are all using a common front-end. It is in part an effort to bring more alignment between the tools.

@leafpetersen
Copy link
Member

I see hints saying that the casts are unnecessary, but I'm guessing one of them will fail at runtime (at least in DDC), correct?

Why do you say that? I haven't tested it, but on quick read through, I don't really see a reason why either would fail.

@davidmorgan
Copy link
Contributor

Someone pointed out that this issue breaks built_value's 'polymorphism' feature:

https://github.com/google/built_value.dart/blob/master/example/lib/polymorphism.dart

As used in this example here:

https://github.com/google/built_value.dart/blob/master/example/lib/example.dart#L42

I can add a way for people to work around it, by leaving off one of the "implements" and just adding the methods by hand ... but it's not pretty, and as noted above, it's surprising that different versions of dart have different features.

Is there any chance of a fix for this when the new frontend arrives, please?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-obsolete Closed as the reported issue is no longer relevant P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) web-dart2js
Projects
None yet
Development

No branches or pull requests

10 participants