Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

generic method types not transferred to constructor calls #433

Closed
Andersmholmgren opened this issue Jan 29, 2016 · 6 comments
Closed

generic method types not transferred to constructor calls #433

Andersmholmgren opened this issue Jan 29, 2016 · 6 comments

Comments

@Andersmholmgren
Copy link

In the example below neither instance of Fred ends up with the expected type arguments.

f1 ends up as Fred<List, Object> and f2 as just Fred

class Fred<A extends Iterable, B extends Object> {}

doIt /*<A extends Iterable, B extends Object>*/ (
    Iterable /*=A*/ a, Object /*=B*/ b) {
  final f1 = new Fred<List /*=A*/, Object /*=B*/ >();
  final f2 = new Fred /*<A, B>*/ ();

  print(f1);
  print(f2);
  tryMe(f1);
  tryMe(f2);
}

tryMe(Fred<Set<String>, int> fred) {}

main() {
  doIt/*<Set<int>, String>*/(new Set<int>(), 'hi');
}

resulting output

dart -c test/client/entity/delme/delme.dart 
Instance of 'Fred<List, Object>'
Instance of 'Fred'
Unhandled exception:
type 'Fred<List, Object>' is not a subtype of type 'Fred<Set<String>, int>' of 'fred'.
#0      tryMe (file:///Users/blah/dart/backlogio_acdart/gitbacklog_root/gissue_root/gissue_client_models/test/client/entity/delme/delme.dart:23:30)
#1      doIt (file:///Users/blah/dart/backlogio_acdart/gitbacklog_root/gissue_root/gissue_client_models/test/client/entity/delme/delme.dart:19:3)
#2      main (file:///Users/blah/dart/backlogio_acdart/gitbacklog_root/gissue_root/gissue_client_models/test/client/entity/delme/delme.dart:26:3)
#3      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:261)
#4      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:148)

@jmesserly
Copy link
Contributor

Nice catch! It looks like you're using Dart VM? Sadly there's not a lot we can do there, until generic methods support is added to the language spec.

(A related issue: DDC hasn't implemented #301 yet, so it doesn't pass "A" and "B" to doIt yet either. But I'm just started looking at this.)

If you'd be willing to run your command line program using DDC and node.js, we can support it sooner. Would that work?

Cheers & thanks for the excellent (as usual) bug report :)

@Andersmholmgren
Copy link
Author

ahhh it appears I am confused. I thought this was all part of a dart wide initiative to add support for generic methods.

In this project I have a polymer dart front end and shelf dart backend. I am adding generic methods to improve code quality by increasing the feedback I get from the analyzer. I'm not expecting to make any particular use of the ddc for this project (although it will be huge for my chances of sneaking in more dart code at work ;-)).

The errors I got were from running pub run test on my unit tests for the UI code.

I had naively assumed that the vm was parsing the // style generics too and that it was just a way to test out generic methods in Dart before finally committing to them in the language spec. I guess that means I should do less guessing and more asking :-/

oh well I guess I need to figure out what to do with all the generic method code I just added. I may still be able to keep it but go with the new Fred /*<A, B>*/ form as it drops the generics entirely in the VM and should avoid the runtime blow ups.

@jmesserly
Copy link
Contributor

Oh, we're definitely working towards generic methods. Analyzer strong mode + DDC is the testing grounds for the enhancement proposal. DDC has a sound type system so we need them sooner than other Dart impls do.

We were hoping that the static type checking would be useful on its own, without necessarily having the runtime checking yet.

In current Dart, if you have something that's conceptually a generic method, and it should return something like Iterable< T >, you'll always get Iterable< dynamic > at runtime. Adding static type checking, without the runtime checks, might help catch some bugs at analysis time. But yeah, when using the comments, it won't affect behavior at runtime, so you won't catch any new bugs there.

@Andersmholmgren
Copy link
Author

ah thanks for clarifying. That is great news. So it is just a matter for me and others to get a handle on where the boundaries lie. I had fooled myself into thinking that the doIt method above was kinda equivalent to (and shorthand for).

class DoIt<A extends Iterable, B extends Object> {

  doIt(A a, B b) {
    final f2 = new Fred <A, B> ();

    print(f2);
    tryMe(f2);
  }
}

main() {
  new DoIt<Set<int>, String>().doIt(new Set<int>(), 'hi'));
}

But I see now that it is not the case in general, only for stuff compiled by the dcc (and to some extent the analyser).

Thanks for taking the time to clear things up

@vsmenon
Copy link
Contributor

vsmenon commented Jan 30, 2016

This is a nice example. We may want to disallow this sort of syntax: new Fred<List /*=A*/, Object /*=B*/ > and only allow a type param to replace dynamic in this context (e.g., new Fred<dynamic /*=A*/, dynamic /*=B*/> would be ok). Otherwise, code may blow up in standard checked mode.

@jmesserly
Copy link
Contributor

this is now tracked at dart-lang/sdk#25637

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

3 participants