Function considered pure with call site annotation, but not with pure_funcs

[rtfeldman]

Bug report or feature request?

Bug report

ES5 or ES6+ input?

ES5

Uglify version (uglifyjs -V)

uglify-js 3.3.20

JavaScript input

function modifyWrapper(a, f, wrapper) {

  wrapper.a = a;
  wrapper.f = f;

  return wrapper;
}

function pureFunc(fun) {
  return modifyWrapper(1, fun, function(a) { return fun(a); });
}

var unused = pureFunc(function(x) { return x; });

The uglifyjs CLI command executed or minify() options used.

uglifyjs test.js --toplevel -mc 'pure_funcs="pureFunc",pure_getters=true,keep_fargs=false,unsafe_comps=true,unsafe=true,passes=10'

JavaScript output or error produced.

This was the output:

var r,n,t;n=r=function(n){return n},(t=function(n){return r(n)}).a=1,t.f=n;

However, there should have been no output because:

1. Since I specified pure_funcs="pureFunc", uglify should consider pureFunc a pure function.
2. pureFunc is only called once, and that result is assigned to an unused variable.

It works as expected if I add a /*@__PURE__*/ annotation at the call site, such that the last line becomes var unused = /*@__PURE__*/pureFunc(function(x) { return x; }); - adding that annotation and re-running uglify results in the correct result (of no output).

However, annotating every pureFunc call site with /*@__PURE__*/ should not be necessary, since pureFunc was listed in pure_funcs!

[kzc]

Confirmed bug.

reduce_vars is performed before the pure_funcs test.

$ bin/uglifyjs t3065.js --toplevel -c pure_funcs=[pureFunc],reduce_vars=1
var fun,f,wrapper;f=fun=function(x){return x},(wrapper=function(a){return fun(a)}).a=1,wrapper.f=f;

$ bin/uglifyjs t3065.js --toplevel -c pure_funcs=[pureFunc],reduce_vars=0

$ 

Strangely, disabling inline and reduce_funcs should have the same effect, but it doesn't appear to prevent the function inlining:

$ bin/uglifyjs t3065.js --toplevel -c pure_funcs=[pureFunc],inline=0,reduce_funcs=0
function modifyWrapper(a,f,wrapper){return wrapper.a=a,wrapper.f=f,wrapper}!function(fun){modifyWrapper(1,fun,function(a){return fun(a)})}(function(x){return x});

[alexlamsl]

@rtfeldman thanks for the detailed report 👍

@kzc we have single-use AST_SymbolRef replacements under reduce_vars 😉

[kzc]

we have single-use AST_SymbolRef replacements under reduce_vars

It ought to be additionally gated by either inline or reduce_funcs - probably the latter. Otherwise one cannot disable function inlining unless reduce_vars is disabled, as evidenced by above.

[alexlamsl]

inline is very different from reduce_vars - one of them eliminates AST_Call altogether, the other merely turns f(...) into (function(...){...})(...)

[kzc]

My last comment was unrelated to pure_funcs. It was referring to not being able to disable this function inlining:

$ bin/uglifyjs t3065.js -bc inline=0,reduce_funcs=0 --toplevel
function modifyWrapper(a, f, wrapper) {
    return wrapper.a = a, wrapper.f = f, wrapper;
}

!function(fun) {
    modifyWrapper(1, fun, function(a) {
        return fun(a);
    });
}(function(x) {
    return x;
});

[kzc]

the other merely turns f(...) into (function(...){...})(...)

Notice that pureFuncs in the example was of type AST_Defun, not a variable. That looks like function inlining to me. Could that be additionally gated by reduce_funcs?

[alexlamsl]

@kzc yes I know what you are talking about - and I'm saying the example you've just pasted above isn't inlined. An inlined function would not be of the form !function(...){...}(...) - the function would be long gone.

[kzc]

So other than disabling reduce_vars altogether, the AST_Defun function pureFunc will always be converted to an anonymous AST_Function and inlined as a parameter regardless of the inline and reduce_funcs setting? Is there any way to toggle that specific behavior off for single-use AST_Defuns?

[alexlamsl]

The only difference between var f = function(...){...} and function f(...){...} is that the latter definition is hoisting - the AST_Node type is just an implementation detail.

[rtfeldman]

Thanks for the quick turnaround on this! 😍

I noticed the example in the original is still reproducible if -m is enabled; I have to leave off -m for it to produce no output when running from ea7b85e.

Is that intended behavior? I saw the commit mentioned "Make sure symbols under pure_funcs are also under mangle.reserved to avoid mangling" but I was actually hoping to mangle them. 😅

Is there an underlying technical reason pure functions can't be mangled?

[alexlamsl]

Is there an underlying technical reason pure functions can't be mangled?

Simply for the fact that pureFunc would go by a different name after mangling, so pure_funcs would no longer recognise and act upon it.

[rtfeldman]

Ohhh, got it! So pure_funcs gets applied after mangling?

Does that mean if I wanted to mangle pure functions, I could run once or twice without mangling to complete all the DCE, and then do one final pass with mangling fully enabled to mangle all the pure functions?

[alexlamsl]

That's one way of doing it, yes.

[rtfeldman]

Gotcha! Is there a more recommended way to do it?

(e.g. a way to run uglifyjs once and get output where the pure_funcs have been both DCE'd and then mangled?)

[kzc]

The only difference between var f = function(...){...} and function f(...){...} is that the latter definition is hoisting - the AST_Node type is just an implementation detail.

To the average user it looks like function inlining - the original function declaration is no longer present. That's what reduce_funcs is supposed to gate.

We'll have to disagree on this point. Enough said.

Simply for the fact that pureFunc would go by a different name after mangling, so pure_funcs would no longer recognise and act upon it.

That's actually a bug/regression. Case in point:

$ cat test.js
(function(){
    function Upper(message) {
        return message.toUpperCase();
    }
    Upper("one");
    var a = Upper("two");
    Upper("three");
    console.log(a);
})();

Compare latest master b82fd0a:

$ cat test.js | bin/uglifyjs -bmc pure_funcs='["Upper"]',warnings=0
!function() {
    function o(o) {
        return o.toUpperCase();
    }
    o("one");
    var e = o("two");
    o("three"), console.log(e);
}();

to 2.x:

$ uglifyjs -V
uglify-js 2.7.5

$ cat test.js | uglifyjs -bmc pure_funcs='["Upper"]',warnings=0
!function() {
    function o(o) {
        return o.toUpperCase();
    }
    var n = o("two");
    console.log(n);
}();

As a workaround --no-rename can be used with latest master:

$ cat test.js | bin/uglifyjs -bmc pure_funcs='["Upper"]',warnings=0 --no-rename
!function() {
    function o(o) {
        return o.toUpperCase();
    }
    var n = o("two");
    console.log(n);
}();

Notice that function Upper was mangled to o with the workaround.