Support `@nosideeffects` all code (not just externs)

[mhevery]

There are many scenarios where the developer knows that code has no side-effects but the CC fails to detect it and retains the code.

CC already has @nosideeffects annotation, but that one only works in externs file. Could we make it work in normal code as well?

[ChadKillingsworth]

We have repeatedly avoided doing that. It's a huge foot-gun.

[mhevery]

Could you share the concerns which make it a foot-gun for better understanding?

[ChadKillingsworth]

/** @nosideeffects */
function IHaveSideEffects() {
   document.body.innerHTML = 'Hello World';
}
IHaveSideEffects(); //this gets removed - user lied to the compiler.

Trivial case. This becomes VERY hard to debug the farther apart the call is from the definition. It's also not always clear to users that the function has side effects.

In general, if an annotation can be mis-used, it will be.

[mhevery]

Sure, but that is a developer bug, like many other bugs which developer can make. Alternative is that we are retaining KBs of code which are not needed and we have no tools to remove them.

[ChadKillingsworth]

There have been some strategies discussed regarding this. See #1746. However none have come to fruition. It's a huge balancing act.

[dimvar]

@mhevery I definitely agree that we should be doing better with the examples you brought up recently. We do want to eventually be able to remove this code, by improving the optimizations. This is where we would like to make improvements, e.g., by moving the side-effect calculation in the optimization loop (currently it can't happen because the pass is too slow).

But as Chad said, we don't want to solve this by introducing the annotation in the code. Optimization directives will be misused and the bugs will be very hard to diagnose. We are fine sacrificing some code size to avoid introducing even more complexity to the compiler.

[alexeagle]

I thought we had rough agreement with @blickly that after adding a warning class for usage of @nosideeffects in non-externs code, we could allow this.
Angular needs to tree-shake sooner, rather than wait for eventual improvements to the side-effect calculations. We have a great advantage at this moment, that closure compiler can do better than any other bundler/tree-shaker right now and there is a lot of interest.

[tadeegan]

The compiler side effects computation could definitely use some improvement.

As an alternative to adding "@nosideeffects" someone could invest time into making the PureFunctionIdentifier more accurate. For example, the following function "f" would be marked as having side effects:

function f() {
  var a = 3;
  a += 5;
  return a;
}

Note that the severity of this is probably reduced by the inline variables pass.

See:

closure-compiler/src/com/google/javascript/jscomp/PureFunctionIdentifier.java

Line 58 in de36c96

* <p>TODO: This pass could be greatly improved by proper tracking of locals within function bodies.

And:

closure-compiler/src/com/google/javascript/jscomp/NodeUtil.java

Line 4238 in 26b48d2

static boolean evaluatesToLocalValue(Node value) {

Tests to be improved:

closure-compiler/test/com/google/javascript/jscomp/PureFunctionIdentifierTest.java

Line 947 in de36c96

// TODO(johnlenz): Not yet. Propagate local object information.

closure-compiler/test/com/google/javascript/jscomp/PureFunctionIdentifierTest.java

Line 1321 in de36c96

"i();" // With better locals tracking i could be identified as pure

closure-compiler/test/com/google/javascript/jscomp/PureFunctionIdentifierTest.java

Line 697 in de36c96

// local var, not yet. Note we do not handle locals properly here.

Perhaps I should open a new bug for that, or maybe there already is one...

[NMinhNguyen]

Sorry to be commenting on an old thread, but this is the only one I've found to be remotely relevant to the question I am about to ask. Babel 7 now emits #__PURE__ annotations in order to improve tree-shake-ability when the transpiled output is consumed by Uglify. References: babel/babel#6209, babel/babel#5632, mishoo/UglifyJS@1e51586, and the side_effects compiler option in Uglify's README. I was wondering if Google Closure Compiler also supports or plans to support this annotation at some point in the future?