google / google/closure-compiler

Optimizing away unused function parameter produces redundant comparison !(void 0 >= b)

Open
#3,223 3 comments 0 reactions 0 assignees View on GitHub
internal-issue-created P3 triage-done
Dominant language
JavaScript
Stars
7.7k
Forks
1.2k
Avg merge
2d 12h
Merged PRs (30d)
6

Description

Consider the following function:

```js
/**
* Scans array of non-zero elements in range [start, end[
* and returns the index where value x is found,
* or -1 otherwise.
* You can omit 'end' parameter to scan the range [start, inf[
* Or you can also omit 'start' parameter to scan whole array
* @param {number} x
* @param {number=} start
* @param {number=} end
* @return {number}
*/
function find(arr, x, start, end) {
for(var i = start|0; !(i >= end); ++i) {
if (arr[i] == x) {
return i;
}
if (!arr[i]) {
break;
}
}
return -1;
}
```

here the intent is that the function can be called with optional end parameter, or optional both start+end parameters.

This function `find()` is provided as sort of a generic library function, and its implementation does not know which of the three forms (start + end, only start, no start or end) of the function the user of the library will be using. But Closure does! (or at least it tries)

So if application developer never called this function by providing an end parameter, it would be great to have Closure optimize out the check `!(i >= end)` completely, because `end` is always `undefined`, so when `i` is known to be a number, `!(i >= end)` is always true.

If application developers calls this with

```js
var a = new Uint8Array(4);
a[0] = 1;
a[1] = 1;
a[2] = 2;
a[3] = 1;

console.log(find(a, 2, 0));
console.log(find(a, 1, 1));
```

then Closure does recognize that `end` is always `undefined`, and it is very much on board with the `{number=}` annotation that suppresses it from issuing any warnings, but instead of resolving the result, it generates something unexpected:

```js
function a(e, f) {
for (var d = c, b = f | 0; !(void 0 <= b); ++b) {
if (d[b] == e) {
return b;
}
if (!d[b]) {
break;
}
}
return -1;
}
```

`void 0` is an abbreviation for `undefined`, `void 0 <= b` returns always `false`, so `!(false)` is always true, so `!(void 0 <= b)` would be redundant here.

Any chance it would be possible for Closure to do this kind of optimization? This is something that occurs in practice in Emscripten C/C++ <-> JS UTF-8 string marshalling functions, which get generated to practically every program compiled with Emscripten, and I am trying to make sure that these primitive exposed functions carry absolutely minimum weight to generated code.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.