Generic method prototype syntax doesn't map well to checked mode

[vsmenon]

The following is adapted from dart-archive/dev_compiler#433

This code checks under strong mode, but fails in checked mode:

List/*<T>*/ makeList/*<T extends num>*/(num/*=T*/ val) {
  var list = new List<num /*=T*/>(); // Runtime error here
  list.add(val);
  return list;
}

void main() {
  List<int> l = makeList/*<int>*/();
}

with the following error on the VM:

Unhandled exception:
type 'List<num>' is not a subtype of type 'List<int>' of 'l'.

In general, allowing A /*=T*/ where A != dynamic in any place a type is reified is dangerous and may lead to inconsistencies as above. Note, if the programmer wrote new List<dynamic /*=T*/>() (or new List/*<T>*/()), there would be no problems.

[jmesserly]

EDIT: see my comment below for a TL;DR

So this was (sort of) by design. Or at least, that's been the conclusion when I raised this issue a few times in the past.

Currently, generic methods attempt to cohere with the future spec language (let's call it "Dart 2.0"). We're assuming that Dart will implement generic methods (in all modes), and once it does, everything will be golden. Also, we assume that in the implementation too. So the comment form was implemented in a very easy-to-rip out way in the scanner/parser.

But if we don't want to wait, then we can tackle this. Hopefully we don't sink too much time, as it may very well be throw-away code.

By far the most egregious hole is you can actually replace a type anywhere in your code :). So you can write String/*=int*/ x; and it will be happy. Would not have been my chosen design, but it does have the benefit that after scanner/parser, all of the generic method implementation in Analyzer is identical with or without comments. That implementation ties our hands a bit, as we can't really tell that a type was replaced.

Other issues: List<num/*=T*/> would definitely be a problem. is S/*=T*/ is also a problem, for any S including dynamic, e.g. is dynamic/*=T*/ is bad. as S/*=T*/ is probably okay in some cases (e.g. as AstNode/*=T*/ where T extends AstNode), as long as T <: S, then we're just more restrictive in strong mode. Parameters like num/*=T*/ might be okay, but we'd have to think about the function typing for a tear off (bivariance might save us?).

(We also instantiate the method using the upper bound (here num), not dynamic (as dynamic would immediately be an error in strong mode). That might be okay. Also, you can get universal types, not default instantiation, for generic method tear offs/generic functions, in the static type system. Probably that one is okay too.)

[jmesserly]

Notes from discussion with @vsmenon:

• we'd have to disallow is T regardless of comment form or not, until we get a VM impl of generic methods that reifies T. So we need CodeChecker logic for that.
• we could restrict /*=T*/ to only explicit or implicit dynamics. This should be simple to implement (entirely in the parser). It may cause some type checks to weaken in checked mode, for example if you had AstNode node = ... you'll now want to have var/*=T*/ node = ...
• need to think about if this is enough, but it should cover most cases.

CC @leafpetersen :)

[jmesserly]

we could restrict /*=T*/ to only explicit or implicit dynamics.

Oops, this won't work. We can't regress min/max:

num/*=T*/ min/*<T extends num>*/(num/*=T*/ x, num/*=T*/ y);

So we'd need to know the difference between a type annotation grammar location and a reified type position. Not sure if the parser has that much information, but maybe it does.

This is related to restricting /*=T*/ to only inside generic methods. It's hard in the current implementation to do that, without adding state to the parser. We could but it makes me nervous to add complexity for a temporary prototype comment syntax.

[munificent]

Updated the docs to warn users about this pitfall and encourage using dynamic to avoid it.

[munificent]

https://codereview.chromium.org/1716593002/