google / closure-compiler

A JavaScript checker and optimizer.
https://developers.google.com/closure/compiler/
Apache License 2.0
7.38k stars 1.15k forks source link

Assigning a variable while passing to a function fails AC #4129

Closed alexmalcoci closed 9 months ago

alexmalcoci commented 11 months ago

Passing a variable as an arg to a function, while at the same time assigning the variable makes the advanced compilation think the two args are supposed to be equal, when they are not.

Below is a distilled example of a real use case:

Advanced compilation example ```js function substr (value, begin, end) { return value.slice(begin, end) } // assigning to window so it doesn't get optimized away window.bug = function (s, i) { return substr(s, i, i = 5); } ``` `java -jar closure-compiler-v20230802.jar --js in.js --formatting PRETTY_PRINT --compilation_level ADVANCED` => ```js window.g = function(b, a) { var c = a = 5; return b.slice(a, c); }; ```

Notice how the c and a end up with 5, which makes the slice return an empty string.

Simple compilation works fine because it keeps the 2 variables separate:

Simple compilation example ```js function substr (value, begin, end) { return value.slice(begin, end) } // assigning to window so it doesn't get optimized away window.bug = function (s, i) { return substr(s, i, i = 5); } ``` `java -jar closure-compiler-v20230802.jar --js in.js --formatting PRETTY_PRINT --compilation_level SIMPLE` => ```js function substr(a, b, c) { return a.slice(b, c); } window.bug = function(a, b) { return substr(a, b, 5); }; ```

Interestingly, advanced compilation without the extra function works fine too

Advanced compilation example without extra function ```js // assigning to window so it doesn't get optimized away window.bug = function (s, i) { return s.slice(i, i = 5); } ``` `java -jar closure-compiler-v20230802.jar --js in.js --formatting PRETTY_PRINT --compilation_level ADVANCED` => ```js window.g = function(a, b) { return a.slice(b, 5); }; ```
lauraharker commented 11 months ago

Drive-by comment: this might be the same as https://github.com/google/closure-compiler/issues/4115 but I haven't investigated further.

alexmalcoci commented 11 months ago

Thanks @lauraharker it does seem like the same issue. This scenario is hard to describe and search for duplicates. But that issue is 2 months old, is anyone looking into this? Is there a way to bump it? It's kind of a blocker for us right now.

lauraharker commented 11 months ago

@rishipal was looking into that issue and it's on our list of high priority things to work on. On our internal issue tracker I see Rishi added some updates a few weeks ago, where he had a potential fix but then realized there were some additional edge cases.

alexmalcoci commented 10 months ago

@lauraharker @rishipal I took a stab at fixing the issue. Added a bunch of tests too. https://github.com/google/closure-compiler/pull/4131

rishipal commented 10 months ago

PTAL at my comments on https://github.com/google/closure-compiler/issues/4115.

rishipal commented 9 months ago

This is fixed with bebf834e32d61a14baa608bc49b4081ce0b1ecbc.