dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.26k stars 1.58k forks source link

dart2js: size / readability low hanging fruit #2: generate || and && #3254

Closed rakudrama closed 9 years ago

rakudrama commented 12 years ago

|| and && expand to huge amounts of code. Code with this structure should be recognized and collapsed to something more readable.

Example:

Isolate.$defineClass("HashMapImplementation", "Object", ["_numberOfDeleted", "_numberOfEntries", "_loadLimit", "_values", "_keys?"], {  ...  forEach$1: function(f) {   var length$ = $.get$length(this._keys);   for (var i = 0; $.ltB(i, length$); i = i + 1) {     var key = $.index(this._keys, i);     var t0 = !(key === (void 0));     if (t0) {       var t1 = !(key === $.CTC21);     } else {       t1 = t0;     }     if (t1) {       f.$call$2(key, $.index(this._values, i));     } else {     }   }  },

--->  forEach$1: function(f) {   var length$ = $.get$length(this._keys);   for (var i = 0; $.ltB(i, length$); i = i + 1) {     var key = $.index(this._keys, i);     if (!(key === (void 0)) &&         !(key === $.CTC21)) {       f.$call$2(key, $.index(this._values, i));     }   }  },

For comparison, frog was much closer to the original (the weird comparison against const$0000 is because (1) frog allows null and undefined for Dart null and (2) frog doesn't know const$0000 is non-null).

HashMapImplementation.prototype.forEach = function(f) {   var length = this._keys.get$length();   for (var i = (0);    i < length; i++) {     var key = this._keys.$index(i);     if ((null != key) && ((null == key ? null != (const$0000) : key !== const$0000))) {       f(key, this._values.$index(i));     }   } }

kasperl commented 12 years ago

Set owner to ngeoffray@google.com. Added Accepted label.

DartBot commented 12 years ago

This comment was originally written by ngeoffray@google.com


Fixed in https://chromiumcodereview.appspot.com//10495013


Added Fixed label.

rakudrama commented 12 years ago

Nice!

Any idea why this one didn't reduce:

$._isWhitespace = function(c) {   $0:{     if (!(32 === c)) {       if (!(9 === c)) {         var t1 = 10 === c || 13 === c;       } else {         t1 = true;       }     } else {       t1 = true;     }     if (t1) {       return true;     }   }   return false; };

DartBot commented 12 years ago

This comment was originally written by ngeoffray@google.com


That's because we're not recognizing nested logical operations. Fix under review: https://chromiumcodereview.appspot.com/10539042