Is the advanced mode guaranteed to preserve references to *global* variables?

[Sébastien Doeraene]

Hello,

We're using Closure as one of the many steps in optimizing Scala.js code (I'm the author of Scala.js). So far we avoided "naked" references to global variables, so instead of emitting `Foo` we would emit `$g['Foo']`, where `$g` is initialized with a reference to the global object. This allowed us to refer to any global variable without needing a dedicated externs file.

For obscure reasons, we now really need to emit `Foo` as a naked reference. I assumed I was going to need an externs file to prevent Closure from renaming it--as well as from renaming other stuff into `Foo` and hence shadowing the global one--, but experiments so far suggest that this is, in fact, not needed. Our entire test suite passes without generating externs. We can also observe that nothing goes amiss in this example online:
https://goo.gl/qOKIQM (make sure to select Advanced before compiling).
The code is:

// ==ClosureCompiler==
// @compilation_level ADVANCED_OPTIMIZATIONS
// @output_file_name default.js
// ==/ClosureCompiler==

// ADD YOUR CODE HERE
function hello(name) {
  alert('Hello, ' + name + a + Math + xyz);
}
hello('New user');
hello('Old user');

... // a few more like that

hello('New user');
hello('Old user');

and the generated code is:

function b(c){alert("Hello, "+c+a+Math+xyz)}
b("New user");b("Old user");...;b("New user");b("Old user");

Two observations:

• References to the global variables `a`, `Math` and `xyz` were not renamed.
• The compiler avoided using `a` as the renamed version of other things (the function and its arguments are called `b` and `c`), so that the reference to the global `a` is not shadowed by more local declarations.
  

(if we change `a` into `x` in the original code, the compiler does use `a` as the renamed version of the function)

This is all *very good* for us, as it seems like we do not need to generate externs on the fly for references to global variables.

The question: can this behavior be relied on? Is the advanced mode guaranteed to preserve references to global variables, even in the absence of externs?

The documentation of the Advanced mode is not very specific about this case, but the actual compiler seems to consistently protect global variables in this way. Hence my question.

Thanks in advance for your insights, and also for your great job!

Cheers,

Sébastien

[Chad Killingsworth]

The online web service runs in "third party" mode. It assumes that any undeclared variable reference is external (among other things). You can enable that in other distributions using the undocumented "--third_party" flag. https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/CommandLineRunner.java#L261 

However I would caution against this long term. A better solution might be to run the compiler with verbose warnings on, and then using the output to create a whitelist file for expected errors: https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/CommandLineRunner.java#L623-L627

Of course the best solution would be to have an externs file. Then you'll get the best type safety and optimizations.

Chad Killingsworth

[Sébastien Doeraene]

Hello,

Thank you for your answer. And sorry for the delay in answering, I was expecting the answers to go straight to my inbox but they went through the "digest" mechanism of Groups anyway.

In practice, we do not use the Web interface. We use the Java API directly. In fact, we even construct the JS ASTs ourselves and hand them in to Closure as is. This means we construct the `CompilerOptions` instance ourselves, here:
https://github.com/scala-js/scala-js/blob/b1824d72fba183aa7f2268d1ca79ac968532daac/tools/jvm/src/main/scala/org/scalajs/core/tools/linker/backend/closure/ClosureLinkerBackend.scala#L172-L185

I tried to find my way inside the Closure sources to figure out if that had the same effect as the "third-party" mode, but it's unclear. All that the `--third_party` seems to do is ensure that the `codingConvention` is a `DefaultCodingConvention`, declared at https://github.com/google/closure-compiler/blob/b85c29855a714cae446f61c8a7b2ccaac747722b/src/com/google/javascript/jscomp/CodingConventions.java#L336. In our case it seems the `options.getCodingConvention()` returns `null`. I am not sure yet whether that negatively impacts the global variable renaming issue.

Do you have more insights about this? In particular, could anyone point me to the code that reads the relevant CodingConvention field and takes a decision based on that?

> Then you'll get the best type safety and optimizations.

We don't need the type safety of Closure: we're compiling from Scala, which has a more elaborate type system than Closure to begin with. ;) As for optimizations, I'm not sure how *more* externs would provide more optimizations. On the contrary, we try to restrict our externs to the bare minimal. For example if we access the property `x` of an external JS object `o`, we emit `o['x']` and no externs instead of `o.x` and an externs declaring `.x`. That allows Closure to rename other properties *to* `x`, which it could not do if we declare `.x` in an externs file.

Cheers,
Sébastien

[Chad Killingsworth]

I'm pretty familiar with Scala. I work at a Scala shop.

We have lots of passes that decision off of the coding convention: https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/RenameProperties.java#L336 - I'd be highly surprised if yours truly was null.

If you just use standard property renaming, then you are correct: the more externs you have the less property renaming. However, the standard build of the compiler enables type based optimizations. We have multiple optimization passes that use type information. A compiler will rename a property on an object, even if that property name exists on an extern, provided it can determine the two are not related. But if you aren't currently passing type information to the compiler, you won't be able to take advantage of any of that.

The other effect you are seeing is potentially one of the back-off strategies of property renaming. At least in some cases if the compiler sees o['Foo'], it won't rename o.Foo as that is a common mistake. Of course the quoted property has to be a string literal for that to function.

To sum up though, the only supported way to block renaming is to have an extern with that property, or to quote the property. Using anything else would be depending on an internal feature of the compiler that could change at any time.

Chad Killingsworth

[Sébastien Doeraene]

> I'd be highly surprised if yours truly was null.

I'm pretty sure it gets set to something non-null at some point by some part of the compiler, but at the time we create the `CompilerOptions`, it's definitely `null`. I couldn't figure out where that is done, though.

> To sum up though, the only supported way to block renaming is to have an extern with that property, or to quote the property. Using anything else would be depending on an internal feature of the compiler that could change at any time.

I'm really not concerned about properties. We quote all external properties, and don't quote any of our internal properties, and it's very good like that: our type system allows us to precisely know, with 100% accuracy, what is an external reference and what isn't--it's more precise than Closure's "provided it can determine the two are not related"; we can *always* know whether they are related. We don't plan to change anything about how we handle properties.

In this thread I'm concerned about the global variables, which live in a completely different scope.

Cheers,
Sébastien

[Chad Killingsworth]

we now really need to emit `Foo` as a naked reference

But property-renaming is in play.

Generating an extern for your global objects (not properties) will not effect renaming and is the only safe way to guarantee the behavior you desire. In addition, if you rely on an unquoted reference to not be renamed, that is also dangerous and could also break at any time.

Chad