@nosideeffects annotation and

175 views
Skip to first unread message

al...@mappable.com

unread,
Feb 5, 2016, 5:10:45 PM2/5/16
to Closure Compiler Discuss
Hi All,

From the documentation at https://developers.google.com/closure/compiler/docs/js-for-compiler, it is implied that calls to functions with the @nosideeffects annotation will be removed if their return value is not used.

However, when I try to take advantage of this functionality, instead I get a warning "Suspicious code. The result of the extern function call 'namespace.log' is not being used.", but the code is still present within the compiled output.

I'm compiling in SIMPLE_OPTIMIZATIONS mode, via plovr. Is no-side-effect code only removed in advanced mode? Is there an option I can pass to plovr or the compiler to enable that feature in simple mode?

If it matters, I'm using the following versions:

plovr built from revision af7e74281eb347c743ef8209a53ec8f9da011d1f

Revision numbers for embedded Closure Tools:

Closure Library:    021ed5b313da392a7139d0d3cf7011999ac8b672

Closure Compiler:   39bb337fdd742e6318f7e38bce33663b5ed50472

Closure Templates:  50a0b6950bbe65cdd9d611db2e4ad690c0de4318


-Alex

Chad Killingsworth

unread,
Feb 5, 2016, 7:02:33 PM2/5/16
to Closure Compiler Discuss
There are a few extern methods that you normally consider being side-effect free but actually have hidden side effects. The prime example is using document.createElement in html5 shims. The effect you are seeing is the same heuristic that is used for properties in that calls to a side effect free extern method where the return type is not used are flagged as potentially utilizing hidden side effects. Example:

noSideEffectExternMethod(); //This statement generates a warning and the code is kept.
var foo = noSideEffectExterMethod(); //This statement does not generate a warning and is removed.

In practice this works extremely well for property access (such as element.offsetWidth), but has not demonstrated the same benefits for functions. That's mainly due to the fact that the pass doesn't utilize type information and so many methods can't take advantage of it. This includes calls to document.createElement as that requires type information since the method is defined in the externs as Document.prototype.createElement.

If it's causing harm, I'm not opposed to removing it.

Chad Killingsworth

Nick Santos

unread,
Feb 5, 2016, 7:23:00 PM2/5/16
to closure-compiler
@nosideeffects is for code that does not have side effects.

Logging, by definition, has side-effects.

When we designed the feature, we were aware that people would lie to
the compiler and use this annotation to mean "please remove calls to
this function at compile-time." It isn't really designed to do this,
and is a problematic fit for this use-case.

So we put deliberate limits on @nosideeffects to prevent people from
doing this. This is one of those limits we put in -- the compiler
detects that you're lying, so warns and backs off.

If you want to have logging that is stripped at compile-time, you
really should use compile-time constants. See
https://github.com/google/closure-library/blob/master/closure/goog/log/log.js
for a good example of this pattern.
> --
>
> ---
> You received this message because you are subscribed to the Google Groups
> "Closure Compiler Discuss" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to closure-compiler-d...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/closure-compiler-discuss/dd815f58-3f25-4c51-b7d7-66bf8210ac7e%40googlegroups.com.
>
> For more options, visit https://groups.google.com/d/optout.

Chad Killingsworth

unread,
Feb 5, 2016, 7:47:21 PM2/5/16
to closure-compiler

Also, it is somewhat difficult to force the compiler to keep such hidden side-effects without this behavior. Closure-library has a sink method, but it of course requires use of closure-library.

Chad Killingsworth


al...@mappable.com

unread,
Feb 6, 2016, 11:23:37 AM2/6/16
to Closure Compiler Discuss
Thanks for the info, that helps a lot.

Now I have a question which is more philosophical than practical. What is the point of having the @nosideeffects annotation if the compiler can already detect which functions have side-effects and which don't, and overrides explicit use of it?

Chad Killingsworth

unread,
Feb 6, 2016, 9:00:52 PM2/6/16
to Closure Compiler Discuss
The compiler can only detect side effects if it can see the implementation. It cannot determine this for externs - and the @nosideeffects annotation is only supported in externs for this reason. I'll add a warning for using the @nosideeffects annotation in source code.

Chad Killingsworth

al...@mappable.com

unread,
Feb 7, 2016, 12:21:20 AM2/7/16
to Closure Compiler Discuss
Hmm. I was only using the @nosideeffects annotation in my externs file.

In fact, even if I have a function in an external file that has no side effects at all, it still isn't excluded from the compiled output. Here's a simple example, built with Closure Compiler off the latest master, commit 

b1ea083.


Note that if I assign the result of the call to ns1.stringify to an otherwise unused local variable, I get the exact same result but with no build warning.


$ tail -n +1 *

==> index.html <==

<html>

<body>

</body>

<script src="lib.min.js"></script>

<script src="test.min.js"></script>

</html>


==> lib.js <==

var ns1 = {};

ns1.stringify = function(arg) {

    return '' + arg;

}


==> externs.js <==

var ns1 = {};


/**

 * @params {*} arg

 * @nosideeffects

 */

ns1.stringify = function(arg) {};


==> test.js <==

(function() {

    var str = "Hello World!";

    ns1.stringify(str);

    var el = document.createElement('div');

    el.innerHTML = str;

    document.body.appendChild(el);

})();


==> build.sh <==

#!/bin/sh


java -jar $CLOSURE_PATH \

    --compilation_level WHITESPACE_ONLY \

    --js_output_file lib.min.js \

    --js lib.js;


java -jar $CLOSURE_PATH \

    --compilation_level SIMPLE \

    --externs externs.js \

    --jscomp_warning uselessCode \

    --js_output_file test.min.js \

    --js test.js;


$ ./build.sh 

test.js:3: WARNING - Suspicious code. The result of the extern function call 'ns1.stringify' is not being used.

    ns1.stringify(str);

     ^


0 error(s), 1 warning(s)


$ tail -n +1 *.min.js

==> lib.min.js <==

var ns1={};ns1.stringify=function(arg){return""+arg};


==> test.min.js <==

(function(){ns1.stringify("Hello World!");var a=document.createElement("div");a.innerHTML="Hello World!";document.body.appendChild(a)})();

Alex Kerfoot

unread,
Feb 7, 2016, 12:30:14 AM2/7/16
to closure-comp...@googlegroups.com
Apologies for the formatting in that last email. It looks like pasting the commit SHA copied from the OS X Terminal included some hidden formatting that wasn't apparent in the message composer.

--

---
You received this message because you are subscribed to a topic in the Google Groups "Closure Compiler Discuss" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/closure-compiler-discuss/ZH08VeWjTHo/unsubscribe.
To unsubscribe from this group and all its topics, send an email to closure-compiler-d...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/closure-compiler-discuss/17ba694a-31c1-4027-aa13-a01260299629%40googlegroups.com.

Chad Killingsworth

unread,
Feb 7, 2016, 8:42:33 AM2/7/16
to Closure Compiler Discuss
Right. This is back to the original discussion. The compiler recognizes that a call of the exact form:

noSideEffectExternMethod(args); // return value is not assigned.

Is probably either a mistake or like document.createElement may have hidden side effects. Because of the hidden side effect case, the compiler warns and then ensures that the call is not removed. If you assign the return value to a variable and don't use the variable, both will be removed without warning. It's not a perfect heuristic, but has served very well for a long time.

Here's the original discussions (they predate the github move):
Chad Killingsworth

Chad Killingsworth

unread,
Feb 7, 2016, 8:58:21 AM2/7/16
to Closure Compiler Discuss
Note that if I assign the result of the call to ns1.stringify to an otherwise unused local variable, I get the exact same result but with no build warning.

Oh wow - this line just registered with me. That is indeed a bug - and I've recreated it. https://github.com/google/closure-compiler/issues/1506

Thanks for finding this.

Chad Killingsworth

Chad Killingsworth

unread,
Feb 7, 2016, 7:39:33 PM2/7/16
to Closure Compiler Discuss
After experimenting with this further, I believe this is a disambiguation problem - if you use a property named "foo" instead of "stringify" the method is successfully removed.

Chad Killingsworth

Alex Kerfoot

unread,
Feb 8, 2016, 8:40:50 PM2/8/16
to closure-comp...@googlegroups.com
Hi Chad,

When I rename the 'stringify' method to 'foo' in my example, I still see the same behavior.

Input:

$ cat externs.js 
var ns1 = {};
/**
 * @params {*} arg
 * @nosideeffects
 */
ns1.foo = function(arg) {};

$ cat test.js
(function() {
     var str = "Hello World!";
     var a = ns1.foo(str);
     var el = document.createElement('div');
     el.innerHTML = str;
     document.body.appendChild(el);
})();

Output:

$ cat test.min.js
(function(){ns1.foo("Hello World!");var a=document.createElement("div");a.innerHTML="Hello World!";document.body.appendChild(a)})();


Chad Killingsworth

--

---
You received this message because you are subscribed to a topic in the Google Groups "Closure Compiler Discuss" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/closure-compiler-discuss/ZH08VeWjTHo/unsubscribe.
To unsubscribe from this group and all its topics, send an email to closure-compiler-d...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/closure-compiler-discuss/f824a0fe-afda-4e28-a8c8-4ac52a7700ca%40googlegroups.com.

Chad Killingsworth

unread,
Feb 9, 2016, 9:22:46 AM2/9/16
to Alex Kerfoot, closure-comp...@googlegroups.com
You have a typo - “params” should just be “param”.

I received the expected output with this command:

java -jar build/compiler.jar -O=ADVANCED -W=VERBOSE --externs test-externs.js --js test.js

Chad Killingsworth | Banno | Senior Software Engineer

Jack Henry & Associates, Inc.

Reply all
Reply to author
Forward
0 new messages