Runtime check on non-group type error for a case where we might really have a ground type

[jacob314]

It would be nice if the error occurred when attempting to instantiate Foo.
Foo should be fine.

This example comes from the protobuff library. pb_list.dart

Repro:

class Foo<E> {
  void validate(E e) {
    if (e is! E) { 
      /* DDC:severe: InvalidRuntimeCheckError, Invalid runtime check on non-ground type E */
      throw new ArgumentError(
           'Value ($e) is not of the correct type');
   }
  }
}

[jmesserly]

@leafpetersen thoughts on this? anything we need to do here?

[leafpetersen]

This pattern comes up in a few places - the SDK uses this pattern, see #103 . Options are:

1. reject the class definition statically
2. reject an instantiation at non-ground type statically
3. reject the is check dynamically (throw an error if the is check gets executed at non-ground type).

For 1) to be tenable, I think we need to add some helper functions that we can replace the is check with (basically expose the regular dart "is" via a runtime operation which we can implement trivially in regular dart). 3) might be the quickest way forward for now. 2) is ok, but it's a little brittle in that you can write a library which gets no errors, and then someone tries to use your library and instantiates a class at non-ground type, and gets stuck. So I think it's least desirable. 3) has the same basic issue.

So I think that rejecting the code and replacing uses of this with calls to a runtime helper would be the best option, though unfortunately the one that requires the most changes to existing code.

[jmesserly]

hmm, perhaps a variant on 3: reject the is check dynamically, but only if the results would differ from existing Dart rules?

e.g. x is E:

• if x is a List<int> and E is a List<int>: both answers are true
• if x is a int and E is a List<int>: both answers are false
• if x is a List<dynamic> and E is a List<int>: answers conflict, so throw

Not sure how much this would help in practice, but it would at least allow using non-ground type for E under some circumstances, depending on what values you're checking against it.

[leafpetersen]

Yes, that would be the right way to do 3 I think.

[vsmenon]

Merging into #236