Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
rakudrama opened this issue May 26, 2012 · 4 comments
Closed

Comments

@rakudrama
Copy link
Member

|| 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
Copy link

kasperl commented May 31, 2012

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

@DartBot
Copy link

DartBot commented Jun 4, 2012

This comment was originally written by ngeoffray@google.com


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


Added Fixed label.

@rakudrama
Copy link
Member Author

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
Copy link

DartBot commented Jun 7, 2012

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

copybara-service bot pushed a commit that referenced this issue Nov 16, 2022
…ackage_config, path, shelf, term_glyph, test_reflective_loader, webdev, webkit_inspection_protocol, yaml

Revisions updated by `dart tools/rev_sdk_deps.dart`.

cli_util (https://github.com/dart-lang/cli_util/compare/b0adbba..edcf1c3):
  edcf1c3  2022-11-15  Devon Carew  blast_repo fixes (#71)

csslib (https://github.com/dart-lang/csslib/compare/ba2eb2d..34203c0):
  34203c0  2022-11-15  Kevin Moore  blast_repo fixes (#154)

dartdoc (https://github.com/dart-lang/dartdoc/compare/08e3098..dc502d0):
  dc502d08  2022-11-15  Sam Rawlins  Move much PackageWarning logic _out_ of PackageGraph, into the enum. (#3251)
  ad651b15  2022-11-15  Sam Rawlins  Move the e2e source-link test to a unit test (#3254)
  d14c680c  2022-11-11  Sam Rawlins  Make many Strings in DocumentationComment non-nullable (#3243)
  92afb9bb  2022-11-10  dependabot[bot]  Bump github/codeql-action from 2.1.29 to 2.1.31 (#3246)

http (https://github.com/dart-lang/http/compare/6339026..d56141d):
  d56141d  2022-11-11  Brian Quinlan  Upgrade to ffigen ^7.2 and remove unnecessary casts (#820)
  d95a544  2022-11-10  Brian Quinlan  Add a more complete implementation for `URLSessionTask`. (#818)

intl (https://github.com/dart-lang/intl/compare/442193c..a127902):
  a127902  2022-11-16  Kevin Moore  blast_repo fixes (#510)

matcher (https://github.com/dart-lang/matcher/compare/6a9b83b..9051de0):
  9051de0  2022-11-15  Kevin Moore  blast_repo fixes (#197)

mockito (https://github.com/dart-lang/mockito/compare/748e88e..347d3e4):
  347d3e4  2022-11-14  Kevin Moore  blast_repo fixes (#587)

package_config (https://github.com/dart-lang/package_config/compare/cff98c9..abb4aec):
  abb4aec  2022-11-15  Kevin Moore  blast_repo fixes (#127)

path (https://github.com/dart-lang/path/compare/58ba22c..12ce876):
  12ce876  2022-11-14  hellohuanlin  Support more arguments in path.join API (#130)

shelf (https://github.com/dart-lang/shelf/compare/5fd2593..1c21047):
  1c21047  2022-11-11  Devon Carew  update the no-response.yml configuration (#308)

term_glyph (https://github.com/dart-lang/term_glyph/compare/ec7cf7b..822cd5b):
  822cd5b  2022-11-15  Kevin Moore  blast_repo fixes (#29)

test_reflective_loader (https://github.com/dart-lang/test_reflective_loader/compare/ef934b7..52b6753):
  52b6753  2022-11-15  Kevin Moore  blast_repo fixes (#42)

webdev (https://github.com/dart-lang/webdev/compare/22f6271..3ec168f):
  3ec168f  2022-11-15  Anna Gringauze  Add --enable-experience flag and pass it to the expression compiler service (#1794)
  72272dd  2022-11-10  Elliott Brooks (she/her)  Include a settings page for configuring the Dart Debug Extension (#1776)
  73839e7  2022-11-10  Elliott Brooks (she/her)  Log entire exception message instead of first line (#1782)

webkit_inspection_protocol (https://github.com/google/webkit_inspection_protocol.dart/compare/b825c8f..ddb624c):
  ddb624c  2022-11-14  Kevin Moore  blast_repo fixes (#95)

yaml (https://github.com/dart-lang/yaml/compare/fda5b15..f699275):
  f699275  2022-11-15  Devon Carew  Merge pull request #131 from dart-lang/blast_repo-2022_11_15T20_23_04
  8f6a5f7  2022-11-15  Kevin Moore  blast_repo fixes

Change-Id: I5d55d733b7f9256edf4f44229cc810e827c23f7d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/270240
Auto-Submit: Devon Carew <devoncarew@google.com>
Commit-Queue: Kevin Moore <kevmoo@google.com>
Reviewed-by: Kevin Moore <kevmoo@google.com>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants