Skip to content

remove extraneous spaces between ++/+/--/-#1488

Closed
alexlamsl wants to merge 1 commit intomishoo:masterfrom
alexlamsl:issue-1377
Closed

remove extraneous spaces between ++/+/--/-#1488
alexlamsl wants to merge 1 commit intomishoo:masterfrom
alexlamsl:issue-1377

Conversation

@alexlamsl
Copy link
Copy Markdown
Collaborator

@alexlamsl alexlamsl commented Feb 14, 2017 •
edited
Loading

fixes #1377

I used the following to test on IE8, which is a modified version of the new test included in this PR:

<html>
<body>
<script>
function evaluate(exp) {
  try {
    return new Function("var a=1,b=2,c=" + exp + ";return{a:a,b:b,c:c}")();
  } catch (e) {
    return false;
  }
}

document.write("<pre>");
var unary = ["++a", "--a", "+a", "-a", "a", "a--", "a++"];
var binary = ["+", "-"];
for (var i = 0; i < unary.length; i++) {
  var a = unary[i];
  for (var j = 0; j < binary.length; j++) {
    var op = binary[j];
    for (var k = 0; k < unary.length; k++) {
      var parts = [a, op, unary[k].replace(/a/, "b")];
      document.write(parts.join(" "));
      var e = evaluate(parts.join(" "));
      var f = evaluate(parts.join(""));
      if (e.a !== f.a || e.b !== f.b || e.c !== f.c) {
        document.write(" <font color=red>FAILED!</font>");
      }
      document.write("\n");
    }
  }
}
document.write("</pre><p>Test completed.</p>");
</script>
</body>
</html>

@alexlamsl
Copy link
Copy Markdown
Collaborator Author

alexlamsl commented Feb 14, 2017 •
edited
Loading

Here's a page that will generate a table of shortest expressions that would work in a given web browser:

<html>
<body>
<table>
<tr><th colspan=5>Input</th><th>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;</th><th>Best</th></tr>
<script>
function evaluate(exp) {
  try {
    return new Function("var a=1,b=2,c=" + exp + ";return{a:a,b:b,c:c}")();
  } catch (e) {
    return false;
  }
}

function test(expected, exp) {
  var actual = evaluate(exp);
  if (actual.a === expected.a && actual.b === expected.b && actual.c === expected.c) {
    document.write(exp + "</pre></td></tr>");
    return true;
  }
}

document.write("<pre>");
var P = [
  ["", "+", "-"],
  ["++a", "--a", "a", "a--", "a++"],
  ["+", "-"],
  ["", "+", "-"],
  ["++b", "--b", "b", "b--", "b++"]
];
var i = [0, 0, 0, 0, 0], j = 0, p = [];
while (j >= 0) {
  if (j == i.length) {
    j--;
    document.write("<tr><td><pre>" + p.join("</pre></td><td><pre>") + "</pre></td><td>&nbsp;</td><td align=right><pre>");
    var e = evaluate(p.join(" "));
    var m = [0, 0, 0, 0], n = 0, q = [], f = null;
    while (!f && n >= 0) {
      if (n == m.length) {
        f = q.join("") + p[n--];
        if (!test(e, f)) f = null;
      } else if (m[n] < 2) {
        q[n] = p[n] + (m[n++]++ ? " " : "");
      } else {
        m[n--] = 0;
      }
    }
  } else if (i[j] < P[j].length) {
    p[j] = P[j][i[j++]++];
  } else {
    i[j--] = 0;
  }
}
document.write("</table><p>Test completed.</p>");
</script>
</body>
</html>

@kzc
Copy link
Copy Markdown
Contributor

kzc commented Feb 14, 2017

I don't know if it's worth the risk for so little reward.

See my comment in original ticket: #1377 (comment)

kzc
kzc reviewed Feb 14, 2017
View reviewed changes
test/mocha/operator.js
[ "a-- + b--", "a--+b--" ],
[ "a-- + b++", "a--+b++" ],
[ "a-- - ++b", "a---++b" ],
[ "a-- - --b", "a--- --b" ],
Copy link
Copy Markdown
Contributor

@kzc kzc Feb 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two additional cases:

a-- - - --b;
a++ + + ++b;

Copy link
Copy Markdown
Collaborator Author

@alexlamsl alexlamsl Feb 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@kzc
Copy link
Copy Markdown
Contributor

kzc commented Feb 15, 2017

Any slowdown detected in test/benchmark.js with this PR?

@alexlamsl
Copy link
Copy Markdown
Collaborator Author

alexlamsl commented Feb 15, 2017

@kzc I'm aware of how rare this pattern naturally occurring in the wild. Just thought I'd have some fun with this anyway because:

  • I stumbled upon what seems like a minimal change
  • we don't fold things like a + b; b++; at the moment

Thanks for pointing out the missing combinations - I'll update the tests accordingly.

Code master This PR
jquery-3.1.1.js 0.825s 0.810s
angular.js 1.102s 1.113s
math.js 17.322s 17.314s
bootstrap.js 0.202s 0.215s
react.js 1.168s 1.114s
ember.prod.js 2.395s 2.348s
lodash.js 0.621s 0.667s
d3.js 1.615s 1.591s

Except for jquery-3.1.1.js, the minified outputs before and after this PR are identical.

@kzc
Copy link
Copy Markdown
Contributor

kzc commented Feb 15, 2017

LGTM

@alexlamsl alexlamsl mentioned this pull request Feb 15, 2017
Closed
@kzc
Copy link
Copy Markdown
Contributor

kzc commented Feb 15, 2017

I see that writing tests can take 10 times as much time as the fix itself. :-)

@alexlamsl
Copy link
Copy Markdown
Collaborator Author

alexlamsl commented Feb 15, 2017

I see that writing tests can take 10 times as much time fun as the fix itself. :-)

FIFY 😉

alexlamsl added a commit to alexlamsl/UglifyJS that referenced this pull request Feb 18, 2017
@alexlamsl
remove extraneous spaces between ++/+/--/-
94e30e4 
fixes mishoo#1377
closes mishoo#1488
@alexlamsl alexlamsl closed this in ac0b61e Feb 23, 2017
@alexlamsl alexlamsl deleted the issue-1377 branch February 24, 2017 00:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@kzc kzc kzc left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

++ + can be +++

2 participants

@alexlamsl @kzc