Confusing type check error in VM in checked mode

[sethladd]

This code:

import('dart:builtin');

add(String a, String b) {
  return a + b;
}

void main() {
  print(add(1, 2));
}

When run in the VM w/ checked mode, and via the Editor, generates this error:

Unhandled exception:
type 'Smi' is not a subtype of type 'String' of 'a'.
 0. Function: '::add' url: 'file:///Users/sethladd/dart/optionaltypes/optionaltypes.dart' line:3 col:12
 1. Function: '::main' url: 'file:///Users/sethladd/dart/optionaltypes/optionaltypes.dart' line:8 col:12

It is unclear and confusing what "Smi" is. It should read: "type 'int' is not a subtype of type 'String' of 'a'"

[ghost]

What should be reported for following code?

#import('dart:coreimpl');

main() {
  Smi i = 1;
  String s = i;
}

or

Map x = new Map();
String s = x;

type 'HashMapImplementation<Dynamic, Dynamic>' is not a subtype of type 'String' of 's'.

While not always clear, the reported error is precise. A possible solution is to also print all implemented interfaces, but that can be quite verbose.

[sethladd]

It is unclear what a Smi is. It's not in our docs.

[ghost]

Maybe we should always print type 'num(Smi)', 'num(Mint)', 'num(BigInt)' when appropriate. Or fix docs?

[sethladd]

We should avoid Smi, Mint, and BigInt, which are implementation details. Dart has num, int, and double. Frog correctly warns that "int is not of type String" in this case.

[ghost]

Do not confuse static warnings with runtime checks! The later one look at the real type and throw an exception. They are enabled if running in checked mode.

Running frog in checked mode reports:
Failed type check: type Number is not assignable to type String

We can add one-offs for Smi, Mint, Bigint type error reporting, as those will be most likely to cause confusion.

---

Set owner to @sgmitrovic.
Added Accepted label.

[sethladd]

Agreed, static warnings aren't runtime checks. However, I am concerned whenever we let implementation details bleed up to user facing messaging. Another example is when the VM talks about "GrowableArray" which is not in the spec nor in our api.dartlang.org docs. To the best of our efforts, we should hide implementation class names from user facing errors, or document these implementation names so users can track them down and not be surprised.

Thanks for your help with this!

[ghost]

Fixed in r5525 (for integers).

There is now a central place in the VM where we map real type names to user-understandable names. Please continue reporting error usability issues. I do not add mapping too eagerly since these changes hide what type actually caused the error. We need to find a good balance.

The runtime checks know only about the real (implementation) type. E.g., in following code frog reports type "Array is not assignable to type num", while type "Array" is not documented (similar to the GrowableArray issue that you mentioned).

check(int a) { return a; }
void main() => check([1, 2]);

We need to have a consistent story across all Dart implementations.

---

Added Fixed label.

[sethladd]

Thanks for the fix!