PR looks good. Didn't see any obvious issues.

I agree reduce_vars should be false by default, at least for several months of testing in the wild after release.

Could you add some negative tests exercising all the cases where sym.modified = true preventing the optimization?

Could you please document the new option as well?

@kzc thanks for the quick review, will get onto more tests and documentation now.

BTW, I find this case of collapse_vars rather surprising:

var A = 1;
(function() {
  console.log(A - 2);
})();

becomes:

(function() {
  console.log(-1);
})();

Evaluation to -1 is okay, but removal of the top-level var A seems odd as unused flag would have left that untouched.

It is as intended. unused is not consulted by a few optimizations.

Google Closure transforms that example into console.log(-1); in advanced mode.

@kzc I see - thanks for the clarification. I have left out any global/top-level variables with reduce_vars for now as I am concerned with their values being modified externally, but we can always remove that !d.global at a later date if we want to I guess.

Added tests and documentation, PTAL.

 +            if (parent instanceof AST_Unary && (parent.operator === '++' || parent.operator === '--')
 +             || parent instanceof AST_Assign && parent.left === node) {
 +                sym.modified = true;
              }

I see there's a ++ postfix modification test. Could you please add a -- prefix test case?

I appreciate that operators like += and are instances of AST_Assign for which you already have = tests, but could you add one such case of a complex assign op in the modified test?

I have left out any global/top-level variables with reduce_vars for now as I am concerned with their values being modified externally

I wouldn't be concerned with that. If any such variables were modified externally then the reduce_vars optimizations would be invalid anyway.

Added tests for -- prefix, *= compound assignment, and for global/top-level variables. reduce_vars now applies to top-level variables as well.

LGTM

Looks like it'd be a useful feature.

Looking gook but I reviewed the pr from my smartphone and couldn't test it throughoutly.

Just FYI, found one case where reduce_vars would cause suboptimal minification, i.e. .$injectorProvider is longer than the original ['$injector'+v]

I wouldn't worry about it. That's a variation of the issue observed here: #1194

I have a fix but it needs refinement.

Nice, thanks! Merged as 4761d07