Inserting dart.notNull checks by default on a loop iteration variable

[sethladd]

Consider this code:

class Foo {
  String name;
  Foo(this.name);
  String toString() => name;
}

main() {
  for (var i = 0; i < 10000; i++) {
    print(new Foo('Hi $i'));
  }
}

It generates this code:

var loop;
(function(exports) {
  'use strict';
  class Foo extends dart.Object {
    Foo(name) {
      this.name = name;
    }
    toString() {
      return this.name;
    }
  }
  // Function main: () → dynamic
  function main() {
    for (let i = 0; dart.notNull(i) < 10000; i = dart.notNull(i) + 1) {
      core.print(new Foo(`Hi ${i}`));
    }
  }
  // Exports:
  exports.Foo = Foo;
  exports.main = main;
})(loop || (loop = {}));

Why is i wrapped in dart.notNull ?

[sethladd]

I had to use -n int to get rid of those checks. However, I was assuming dev_compiler could just see that i is never null.

[vsmenon]

We're doing very few optimizations right now. This one is worthwhile for readability though.

Note, "-n int" indicates that int should be treated as a non-nullable type and will give a static error if you ever try to assign null to an int-typed location. That's an alternative approach we're looking at to reducing null checks on primitive types.

[sethladd]

readability and performance :)

[vsmenon]

Absolutely, but the current focus is readability, so those bugs will get priority. :-)

[sethladd]

Fair enough. I'm assuming that when we generate readable code, that implies "a human would have written it that way". And I'm assuming that JS engines are getting faster and faster.

I'm very very happy to trade semantics (like all those null checks) for readable and, thus, performant code.

[jmesserly]

:+1: Seth! I think #70 will help here

[jmesserly]

we could also do --trust-primitives flag like dart2js does

[ochafik]

Notwithstanding the upcoming nullability-awareness of the type system, we could easily detect non-nullable locals and drop lots of notNull checks (see this branch).

Going further, we could eliminate the cost of dart.notNull(this.length) for core collections to speed up what probably are the hottest loops by moving the boundary check outside the loop (see this other branch diffbased on the previous: it special-cases int + 0 in the spirit of asm.js' number | 0 coercions). WDYT?

[jmesserly]

@ochafik -- if I understand correctly, the runtime code we inherited from dart2js already has this non-null magic, so your change just teaches DDC to recognize the pattern? If so that sounds like an excellent idea! Definitely send that CL out!