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

calls to the querySelector method are cut off in case of ADVANCED_OPTIMIZATIONS and turned off checkTypes warnings #2365

Closed
nederland074 opened this issue Mar 9, 2017 · 6 comments
Assignees

Comments

@nederland074
Copy link
Contributor

The compilation of a file, which contains only one line:
document.querySelector(".test").some_prop = true;

leads to an empty output when using the compiler with these flags:
java -jar compiler.jar --compilation_level=ADVANCED_OPTIMIZATIONS --jscomp_off=checkTypes --js=in.js --js_output_file=out.js

However, enabling the checkTypes warnings (--jscomp_warning=checkTypes) fixes it and the output is ok: document.querySelector(".test").a=!0;

It doesn't matter if the altered property is a custom "some_prop" or a built-in one like the "className" or "id". Also, the querySelectorAll method doesn't have such an issue and the following code is compiled without problems:
document.querySelectorAll(".test")[0].some_prop = true;

It's reproduced in GCC v20161201.

@nederland074 nederland074 changed the title calls to the querySelector method are cut off in case of ADVANCED_OPTIMIZATION and turned off checkTypes warnings calls to the querySelector method are cut off in case of ADVANCED_OPTIMIZATIONS and turned off checkTypes warnings Mar 9, 2017
@MatrixFrog
Copy link
Contributor

Hm. I wonder if we accidentally made an assumption somewhere that anyone using advanced mode would also be using typechecking.

@tonsky
Copy link

tonsky commented Nov 22, 2017

any progress on this? Ran into it today with both v20170910 and v20171023

@brad4d brad4d self-assigned this Nov 27, 2017
@brad4d
Copy link
Contributor

brad4d commented Nov 27, 2017

I played around some with reproducing this today.
The --print_source_after_each_pass option seems to indicate that peepholeOptimizations pass is removing the whole statement.

Further note regarding the "it doesn't happen with querySelectorAll()" note in the description.
The important difference there is not the method being called, but the [0] array access before
accessing the property.

@ChadKillingsworth
Copy link
Collaborator

In ADVANCED mode, property sets do not have arbitrary side-effects (it is ok to remove property writes if the property is never read), as described here

@brad4d
Copy link
Contributor

brad4d commented Nov 29, 2017

@ChadKillingsworth thanks for that info.
It's a bit more than that, though.

Suppose we have code like this:

someExternFunction().someProperty = something;

someExternFunction() might return a value that is part of global state, so setting someProperty on it affects global state and is unsafe to remove. We should assume that this is the case for extern functions unless type info tells us otherwise.
PureFunctionIdentifier is incorrectly assuming the opposite when it has no type information.

I have a change to fix this in internal review.

@ChadKillingsworth
Copy link
Collaborator

@brad4d Yeah - that was just a reminder for external viewers.

@brad4d brad4d closed this as completed in f88d939 Nov 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants