google / google/closure-compiler

Incorrect Optimization on Constant Properties

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

Description

I discovered an advanced optimization issue introduced in version `v20190325`. The compiler produces a warning about the modification of a constant property, but also produces incorrect optimized code. I feel the warning should be upgraded to an error, or the optimization prevented in this case.

Here is a minimal example to reproduce the issue.

`example.js`:
```
goog.module('example');
exports = {};
exports.nextId = 1;
const getNextId = () => {
const result = exports.nextId;
exports.nextId++;
return result;
}
if (getNextId() == 2) {
console.log("Bad Optimization");
} else {
console.log("Good Optimization");
}
```
With compiler version `v20190301`
```
$ java -jar closure-compiler-v20190301.jar --compilation_level ADVANCED_OPTIMIZATIONS --formatting PRETTY_PRINT --js_output_file=bug.js example.js && cat bug.js && node bug.js
example.js:8: WARNING - constant property nextId assigned a value more than once
exports.nextId++;
^^^^^^^^^^^^^^

0 error(s), 1 warning(s), 96.0% typed
var a = 1, b = a;
a++;
2 == b ? console.log("Bad Optimization") : console.log("Good Optimization");

Good Optimization
```
And then with version `v20190325`
```
$ java -jar closure-compiler-v20190325.jar --compilation_level ADVANCED_OPTIMIZATIONS --formatting PRETTY_PRINT --js_output_file=bug.js example.js && cat bug.js && node bug.js
example.js:8: WARNING - constant property nextId assigned a value more than once
exports.nextId++;
^^^^^^^^^^^^^^

0 error(s), 1 warning(s), 96.0% typed
var a = 1;
a++;
2 == a ? console.log("Bad Optimization") : console.log("Good Optimization");

Bad Optimization
```

Small changes to the example code such as replacing `++` with `+= 1` also prevent the incorrect optimization.

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.