Stringify closures consistently between Dart VM, dart2js, and DDC

[yjbanov]

dart2js and DDC stringify closures differently from the Dart VM. To avoid "browser quirks" in Flutter for web, we'd like the behavior to be consistent.

Example:

void main() {
  print(() {});
  print(fn);
}

void fn() {}

The Dart VM prints:

$ dart print_closure.dart
Closure: () => Null
Closure: () => void from Function 'fn': static.

dart2js output prints:

$ node out.js
Closure 'main_closure'
Closure 'fn'

Dart SDK info:

$ dart --version
Dart VM version: 2.3.1 (Tue May 21 19:28:38 2019 +0200) on "linux_x64"
$ dart2js --version
Dart-to-JavaScript compiler (dart2js) version: 2.3.1

I haven't tried DDC (I don't know how to use it from the command line), but I'm guessing it's not consistent either.

/cc @vsmenon @jonahwilliams

[a-siva]

@yjbanov can you indicate your preference of which form makes sense for your use case.

[vsmenon]

DDC prints:

Closure: () => Null from: () => {
  }
Closure: () => void from: function () {
}

We probably don't want this in any kind of release mode - e.g., dart2js will try to treeshake away type metadata like this.

[vsmenon]

@yjbanov - the upstream example seems a little different - e.g., the onClick doesn't match the patterns here. Is that coming from a Function.toString()?

[matanlurey]

@yjbanov You might also be interested in #36165.

[rakudrama]

Including the type in string returned by toString() has code size implications.
For dart2js to match the information baked into the VM result would increase code size

• Including the function type in the output basically requires all closures to carry the type. For dart2js we want to carry fewer types, i.e. only where they are needed for type tests.
• dart2js minified JavaScript does not retain the original names. Null and void might be special-cased, so I suggest that you work with an example containing a user-defined class with a 'long' (4+ character) name.
• I think all implementations should also use the Dart syntax for function types, not the => pseudo-syntax. I would prefer to defer any changes to function type formatting until it is resolved what all tools should print, both at 'compile' time and 'run' time.

[yjbanov]

@a-siva We'd prefer the Dart VM behavior as the alternative would mean introducing a breaking change to Flutter.

@rakudrama This is relevant for debug builds only. Production builds should optimize for code size and performance. It might be a fine choice to implement this only in DDC and not in dart2js. However, it would mean that we will not be able to run certain tests in dart2js mode.

@matanlurey Thanks for the link! Improving debuggability is always a plus. I'm guessing though that the change requested in #36165 would be a fairly breaking one.

[lrhn]

I'd recommend just using Closure as the toString, and omitting the type entirely. If the user needs the type, they can print closure.runtimeType. Development builds could include source file/line/offset of the function declaration, for easier debugging, but having a different toString between development and production is a good way to get subtle bugs, so I'd prefer if it's only the debugger who can access that information, not the code itself.

[rakudrama]

I would like to close this as infeasible.

We are not going to make them consistent since it costs too much in different ways for the platforms to be both consistent and helpful.

Returning the lowest-common-denominator "Closure" is is just unhelpful. I'm against this because relying on a nice debugger experience is of no help to developers who have to understand crashes in production from logs that contain only the textual error message and stack trace. The message needs to be as helpful as possible.

Even when one can print closure.runtimeType, that does not tell you which closure it is. The code is often such that there is no good place to print(closure.runtimeType), for example, copying a list of closures via myList.forEach(otherList.add) can fail due to a parametric covariance check in add, but all places you might try to inspect are hidden in the runtime and used by lots of other code.