`.toString(16)` is not optimised away if the result is unused

[copy]

The following used to be compiled away (in v20210601, even without the type annotations and the typeof):

// ==ClosureCompiler==
// @compilation_level ADVANCED_OPTIMIZATIONS
// @output_file_name default.js
// ==/ClosureCompiler==

/**
 * @param {number} n
 * @param {number=} len
 * @return {string}
 */
function h(n, len)
{
    let str;
    if(typeof n === "number")
    {
        str = n.toString(16);
    }
    else
    {
        str = n + "";
    }

    return "0x" + str.toUpperCase().padStart(len || 1, "0");
}

/**
 * @param {number} x
 */
window["main"] = function(x)
{
  h(x);
}

It now generates (tested with v20220405 and the service):

 window.main=function(a){"number"===typeof a&&a.toString(16)};

I tend to write code similar to this a lot: dbg_log("x=" + h(x) + " y=" + h(y)), where dbg_log gets optimised away in the release build properly, leaving the (sometimes costly) x.toString(16) operation.

[lauraharker]

Interesting - Closure Compiler has always hardcoded that toString() with zero args has no side-effects here: https://github.com/google/closure-compiler/blob/ee55469d388cd1e132fb9f372fc878818e11ac3b/src/com/google/javascript/jscomp/AstAnalyzer.java#L67-L68

and we also assume it is side-effect free in the standard externs: https://github.com/google/closure-compiler/blob/ee55469d388cd1e132fb9f372fc878818e11ac3b/externs/es3.js#L1196

Not sure why the one-arg call stopped being dead-code eliminated, but maybe we should just extend the zero-arg hardcoding to handle multiple arguments (unless the args themselves have side effects)

[copy]

Invalid radices might be a problem:

> (42).toString(99)
Uncaught RangeError: toString() radix argument must be between 2 and 36
    at Number.toString (<anonymous>)

[lgrachov]

And look what the make all command in terminal said when I try to compile copy.sh/v86:

mkdir -p closure-compiler
# don't upgrade until https://github.com/google/closure-compiler/issues/3972 is fixed
wget -nv -O closure-compiler/compiler.jar https://repo1.maven.org/maven2/com/google/javascript/closure-compiler/v20210601/closure-compiler-v20210601.jar
make: wget: No such file or directory
make: *** [closure-compiler/compiler.jar] Error 1

[niloc132]

@lgrachov that works for me:

2023-06-01 08:30:53 URL:https://repo1.maven.org/maven2/com/google/javascript/closure-compiler/v20210601/closure-compiler-v20210601.jar [12634009/12634009] -> "closure-compiler/compiler.jar" [1]

Is it possible that a proxy was preventing you from loading the url? From time to time maven also has caching issues, can you confirm that https://repo1.maven.org/maven2/com/google/javascript/closure-compiler/v20210601/ shows the closure-compiler-v20210601.jar link as expected, and that your browser can download it?

[lgrachov]

@niloc132 My browser can download it

[spetterman66]

progress?

[brad4d]

I believe @copy was right about why .toString(16) is not optimized away. If the method could throw an exception, then closure-compiler should generally consider it to have side-effects.

One could make arguments for handling cases where the compiler can prove to itself that an exception will never be thrown, but really, the amount of code size improvement one is likely to get from removing these calls is insignificant.

I don't think it is likely that the Google support team will spend time on it, so I'm closing this issue.

However, if someone else wants to have a go at creating a PR, we'll review that.