Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect DCE of assignment to destructered parameter field with ES_2015 or higher #3499

Closed
WearyMonkey opened this issue Nov 4, 2019 · 9 comments
Assignees
Labels
internal-issue-created An internal Google issue has been created to track this GitHub issue triage-done Has been reviewed by someone on triage rotation.

Comments

@WearyMonkey
Copy link
Contributor

WearyMonkey commented Nov 4, 2019

Code sample:

function installBaa({ obj }) {
    obj.baa = 'foo';
}

const obj = {};
installBaa({ obj });
console.log(obj.baa);

Command:

java -jar compiler.jar --js test.js -O ADVANCED --language_out ECMASCRIPT_2015

Version: v20191027

Expected output: (as output by --language_out ECMASCRIPT5)

console.log({a:"foo"}.a);

Actual output:

'use strict';console.log({}.a);

It looks like CC is losing the assignment when the target language is ECMASCRIPT_2015 or higher.

The following non-destructered parameter works as expected:

function installRoutes(x) {
    x.obj.baa = 'foo';
}
@lauraharker
Copy link
Contributor

Created internal Google issue http://b/143889127

@lauraharker lauraharker added internal-issue-created An internal Google issue has been created to track this GitHub issue triage-done Has been reviewed by someone on triage rotation. labels Nov 4, 2019
@lauraharker
Copy link
Contributor

Here's a smaller repro:

const obj = {};

(function() {
  const {obj: objLocal} = {obj};
  objLocal.baa = "foo";
})();

console.log(obj.baa);

becomes with the FOLD_CONSTANTS and COMPUTE_FUNCTION_SIDE_EFFECTS flags:

const obj = {};
console.log(obj.baa);

For whatever reason the peephole passes think the IIFE is a no-op and are deleting it.

@WearyMonkey
Copy link
Contributor Author

@lauraharker

Any update on this issue?

It forces us to introduce Babel into our pipeline to transpile spread operators before passing Closure Compiler.

@lauraharker
Copy link
Contributor

Hi, I'm afraid I don't have an update :( This is on our hotlist of issues to prioritize but no one is working on it at the moment.

I'll try to take a look this week.

@lauraharker lauraharker self-assigned this Jan 30, 2020
@WearyMonkey
Copy link
Contributor Author

Thank you!

@lauraharker
Copy link
Contributor

Update: I have a fix and am in the process of internal code review. This should be fixed in the next release.

@WearyMonkey
Copy link
Contributor Author

WearyMonkey commented Aug 17, 2020

@lauraharker I just tested my sample code in v20200719 and it's still not working?

Did your fix make it into the release?

@WearyMonkey
Copy link
Contributor Author

@lauraharker it looks like your smaller repro is fixed, but not my original one.

@lauraharker
Copy link
Contributor

Sorry about that, I'll reopen this issue.

link to your original repro

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal-issue-created An internal Google issue has been created to track this GitHub issue triage-done Has been reviewed by someone on triage rotation.
Projects
None yet
Development

No branches or pull requests

2 participants