remove non-observable side effects code

[bobzhang]

The output below is correct, a seems to be a global exception id, however, such side effect is not observable, is it possible to remove such code(ideally, I should get an empty output)

// Input 0./caml_exceptions.js
'use strict';
// Input 1./caml_array.js
// Input 2./caml_primitive.js
// Input 3./caml_utils.js
// Input 4./caml_io.js
// Input 5./camlinternalFormatBasics.js
// Input 6./pervasives.js
var a = 0;
a++;
// Input 7./array.js
a++;

[dimvar]

a can be read if this code is executed in some context that uses a. If you want to remove top-level variables from your program, you can wrap it in (function() { ... })() before compiling.

[ChadKillingsworth]

@dimvar This is occurring in ADVANCED mode as well. I think we should fix it. It should be completely eliminated as dead code.

[bobzhang]

Yes, I am testing in ADVANCED mode, a is a renamed variable(non-exported), if you refer a in toplevel, it is probably a mistake

[dimvar]

@ChadKillingsworth I was already assuming that we're talking about advanced mode. It's not easy to have a rule "if a variable has been renamed, then you can assume it's not used by external code". These two passes are independent, and the removal pass doesn't know which variables have been renamed or not.

And it's very easy to wrap the code to get this optimization.

[ChadKillingsworth]

@dimvar I thought we already assumed global variables weren't read externally unless exported.

[nicks]

i'm surprised that RemoveUnusedVars doesn't catch this out of the box, I think it's supposed to efficiently handle the case where a variable is written to multiple times but never read?

[nicks]

ya, i think that if you change RemoveUnusedVars#traverseNode to classify ++ as assigns when the return value is unused, everything would just work correctly. And you wouldn't incur the performance penalty of a full data flow analysis (which i assume is what @dimvar is afraid of)

[dimvar]

You're right. Reopening.