[closure-compiler-discuss] Complain about unused code instead of removing it

74 views
Skip to first unread message

Klaus Reimer

unread,
Apr 23, 2010, 4:10:54 PM4/23/10
to closure-comp...@googlegroups.com
Hello,

is it possible to configure the compiler to complain about unused code
(With a warning or error) instead of removing it? I don't like automatic
removing because this is most likely a bug that the code is unused so I
want to know about it.

--
Bye, K <http://www.ailis.de/~k/>
[A735 47EC D87B 1F15 C1E9 53D3 AA03 6173 A723 E391]
(Finger k...@ailis.de to get public key)


--
Subscription settings: http://groups.google.com/group/closure-compiler-discuss/subscribe?hl=en

Alan Leung

unread,
Apr 23, 2010, 5:31:46 PM4/23/10
to closure-comp...@googlegroups.com
By unused code, did you mean unreachable code or unused symbols?

Garrett Smith

unread,
Apr 23, 2010, 8:03:09 PM4/23/10
to closure-comp...@googlegroups.com
On Fri, Apr 23, 2010 at 1:10 PM, Klaus Reimer <k...@ailis.de> wrote:
> Hello,
>
> is it possible to configure the compiler to complain about unused code
> (With a warning or error) instead of removing it? I don't like automatic
> removing because this is most likely a bug that the code is unused so I
> want to know about it.
>

+1

Problems with GoogCC include:
* doesn't build a scope tree (so can't make a full optimization)
* confuses FunctionExpression and FunctionDeclaration (now that's alarming)
* removes conditional comments (another topic, but still a problem).

Closure Compiler rewrites code to behave differently. If it would
simply make an incorrect suggestion, I could, being capable of
recognizing that for what it is, simply ignore that suggestion.

I do not feel comfortable allowing such a powerful to to rewrite code
in ways that cause observable differences in program behavior. It is
unsafe. See:

http://groups.google.com/group/comp.lang.javascript/msg/c8a8c92da6e11051

Despite the obviously wrong behavior described in that thread, I would
still consider using closure compiler it if would not rewrite my code;
helpful info would help *me* make the refactoring choices. Ideally
GoogCC would provide proper scope analysis.

Nick Santos

unread,
Apr 23, 2010, 8:10:10 PM4/23/10
to closure-comp...@googlegroups.com
On Fri, Apr 23, 2010 at 8:03 PM, Garrett Smith <dhtmlk...@gmail.com> wrote:
> +1
>
> Problems with GoogCC include:
>  * doesn't build a scope tree (so can't make a full optimization)

i don't know what this means. can you elaborate?

>  * confuses FunctionExpression and FunctionDeclaration (now that's alarming)

this was a small bug, and was fixed a long time ago.

>  * removes conditional comments (another topic, but still a problem).

i don't know of any JS compressor that preserves conditional comments
correctly in all situations. If you can name one, then I'd love to see
it.

>
> Closure Compiler rewrites code to behave differently. If it would
> simply make an incorrect suggestion, I could, being capable of
> recognizing that for what it is, simply ignore that suggestion.
>
> I do not feel comfortable allowing such a powerful to to rewrite code
> in ways that cause observable differences in program behavior. It is
> unsafe. See:
>
> http://groups.google.com/group/comp.lang.javascript/msg/c8a8c92da6e11051

again, the bug described in this thread is no longer true.

Nick Santos

unread,
Apr 23, 2010, 11:08:35 PM4/23/10
to closure-comp...@googlegroups.com
PS -- i'm also curious about the answer to Alan's question. I thought
that closure compiler did a reasonable good job about telling you
about unused branches. It doesn't tell you about unused symbols--but
it did warn about that, then you'll get lots of warnings about unused
library code.

Klaus, it would help to have an example of what you're looking for.

Thanks!
Nick

Klaus Reimer

unread,
Apr 24, 2010, 4:23:23 AM4/24/10
to closure-comp...@googlegroups.com
Alan Leung wrote:
> By unused code, did you mean unreachable code or unused symbols?

I mean unused symbols like private fields, methods and functions and
local variables.

I know that some unreachable code is detected but maybe it can be
improved further. I think the closure compiler has a very good
understanding of what's going on in the code so maybe it is possible to
even detect something like this:

var b = false;
if (b) return 1;
return 2;

Eclipse gives me warning when I do something like this in Java. But this
isn't that important for me now. What I really miss is a warning/error
(Make it configurable like all the other checks) when I have unused
symbols in the code.

Klaus Reimer

unread,
Apr 24, 2010, 4:30:34 AM4/24/10
to closure-comp...@googlegroups.com
Nick Santos wrote:
> about unused branches. It doesn't tell you about unused symbols--but
> it did warn about that, then you'll get lots of warnings about unused
> library code.

Why? I think a warning about unused local variables is always safe. They
can't be used from outside. And for the rest it must look for the
"private" annotation (It does this already for access checks). So if
there is a method or field which is marked as private but is not used in
the same file then this is unused code which should produce a warning or
error because it is most likely a bug that the method is not called or
the field is never accessed. When the method is public or protected then
it is ok when it is not used because it could be used (or overriden) in
other projects.

That's the way it works in Java (with Eclipse).

Eclipse even goes a little bit further and checks if a private field is
READ. When it is just used to assign a value to it then it is still
unused because the value of this field is never used.

--
Bye, K <http://www.ailis.de/~k/>
[A735 47EC D87B 1F15 C1E9 53D3 AA03 6173 A723 E391]
(Finger k...@ailis.de to get public key)


Garrett Smith

unread,
Apr 25, 2010, 2:21:30 AM4/25/10
to closure-comp...@googlegroups.com
On Fri, Apr 23, 2010 at 5:10 PM, Nick Santos <nicks...@google.com> wrote:
> On Fri, Apr 23, 2010 at 8:03 PM, Garrett Smith <dhtmlk...@gmail.com> wrote:
>> +1
>>
>> Problems with GoogCC include:
>>  * doesn't build a scope tree (so can't make a full optimization)
>
> i don't know what this means. can you elaborate?
>

In order explain what I mean by scope tree, it is first necessary
understand scope chain. Scope chain and identifier resolution requires
an understanding of execution context and Variable Object (VO).

ECMA-262 editions 3 and 5 are the normative reference for these.

I suppose I can give a brief explanation...

When a function is created, it gets a scope property that is the
calling context's variable object.

When a function is called, a new Variable object (VO) is created. The
VO is populated with the scope chain of that function. When an
identifier is to be resolved in that call, it is resolved against the
VO. If it is not found, the scope of the function is searched.

A diagram of the scope chain follows the example code:

Code:
function a() {
var v = "property of a VO";
return b;
function b() {
return c;
function c(){ alert( v ); }
}
}
a()()();

Diagram:

global
|
a[scope]
|
b[scope]
|
c[scope]

The first call is a call to `a`, "a()". Identifier `a` resolves on the
global object. When function `a` is called an execution context is
entered and a VO is created. Function `b` and variable `v` are added
as properties of the VO and `b` getting a scope of `a`. Next, the two
statements are evaluated:

1) v = "property of a VO"
2) return b;

The first statement resolves identifier `v` on the VO and then assigns
it a value. The second statement resolves identifier `b` on the VO and
returns that to the calling context, which is the global object.

That returned value is used in another CallExpression, i.e. the second
CallExpression in "a()()". The function that is now being called is
the function returned from `a`. A VO is created, identifier `c` is
added to it and passed in the scope chain, which now contains a VO
from the prior call to `b`, the VO from the call to `a`, and the
global object. The function `c` is returned to the calling context,
which is the global object.

The calling context uses that returned value in another
CallExpression, e.g. the third CallExpression in "a()()()". That
function is called. The VO that is created is empty (except for an
unobservable `arguments` object, if that did not get optimized away).
`alert` is called and identifier `v` is resolved up the scope chain.


The call to alert must resolve identifier `v`. It is not found in the
VO of the current execution context, and so the next object in the
scope chain is searched. That is the scope for the function identified
by `c`, which was the VO when `b` was created. That VO has a `c`
property but not a `v` property. The next object in the scope chain is
what was the VO for `a` when it was called. That has two properties:
`b` and `v`. Identifier `v` is resolved there and the Reference to `v`
is returned to `c`. (The base object for v is the activation object
(VO) and the value is "property of a VO")

The execution for `c` completes normally, returning undefined to its
caller (b), which returns undefined to its caller (a), which returns
undefined, completing the stack of execution contexts.

Now that scope chain has been discussed in more detail, a scope tree
should not be a hard concept to grasp.

At the root of every scope chain is the global object. Every scope
chain shares the root. Some scope chains may share scope. In the
example below, when `a` is called, `b` and `c` have the same scope,
passed in from the execution context of the call to `a`.

Code:
function a() {
var q=1, w=2;
return{ b:b, c:c };
function b(){ alert(q); }
function c(){ alert(w); }
}

var anb = a();


Diagram:

global
|
a[scope]
| |
b[scope] c[scope]


All identifiers are used, and so none can be removed, however it is
not difficult to imagine a case where a function could be removed.

The example below was borrowed (with changes) from:
http://code.google.com/p/closure-compiler/issues/detail?id=36#c16

function x(){
var ChocolateFudgeSundae = 3,
crap = 7;
return a;
function a(){
return b();
}
function b(){
return ChocolateFudgeSundae;
}
// function c() { crap++; }
}

x[[scope]]
| |
a[[scope]] b[[scope]]

By looking at the identifiers declared in the body of a and b, some
optimizations can be found.

We know that if x contains an identifier, and that identifier is not
present in any of the functions within x (or nested functions), then
that identifier can be removed.

That identifier is not present in the body of `x`, the body of `a` nor
the body of `b`, then that A warning should be sufficient for that;
that way it is removed by the developer, in full awareness, cleaning
up the original source code (and possibly taking a second look at why
that identifier was there).

e.g.
| WARN: Dead code: identifier `crap` is declared and assigned a value,
but not used.

With Simple Optimizations we can see that `crap` was renamed to `d`:
function x(){function a(){return b()}function b(){return c}var
c=3,d=7;return a};

`ChocolateFudgeSundae` declared in the body of `x` got renamed to `c`
and all references to it (only one, in `b`) were updated. That makes
sense. If that is possible, then was was `d` not removed?

If we remove the reference to identifier `ChocolateFudgeSundae` in
`b`, it can also be determined that the number of references to that
identifier is 0, and so if no calls to eval exist in `x` or any of its
nested functions, recursively, then the identifier can be removed. If
it is possible to lexically analyze FunctionBody for identifiers and
eval (and it is), then that can be done in a BFS-type search of all
functions within.

The example, updated:

function x(){
var ChocolateFugdeSundae = 3,
crap = 7;
return a;
function a(){
return b();
}
function b(){
return;
}
}

Output:
function x(){function a(){return b()}function b(){}var c=3;c=7;return a};

This is totally pointless declaration and assignment of c, followed by
another pointless assignment to `c`. If `crap` can be removed
completely, then so can the assignment expression.

Advanced optimizations results in:

| Your code compiled to down to 0 bytes. Perhaps
| you should export some functions? Learn more

This does not safely remove unused identifiers. Instead, it removes
used identifiers. It is no longer possible to call the function
returned from x.

x()(); // TypeError

Instead, the scope tree should be built internally, so that function
removal can be determined. If it can be determined that `a` returns
the result of calling `b` and that `b` has the same scope as `a`, then
the FunctionBody for `b` can be added inline to where it is used in
`a`, so long as none of the identifiers used in `b` are the same as
those in `a`.

Another example, hinted at prior:
function x(){
var ChocolateFudgeSundae = 3,
crap = 7;
return a;
function a(){
return b();
}
function b(){
return ChocolateFudgeSundae;
}
function c() { crap++; }
}
global
|
x[[scope]
|
+----------+-----------+
| | |
a[[scope]] b[[scope]] c[scope]

c is an unused identifier. Identifier `crap` appears in a dead node of
the scope tree. It can be removed, along with c.


Another optimization that can be done with scope tree is function
calls inlining and identifier inlining.

Take the code below, for example. The first function could be
optimized to intermediate result:-

function x(){
var y = 3;
return a();
function a(){
return y; // result of calling `b` inlined.
}
}

Identifier `y` can be inlined, resulting in:-

function x(){
return a();
function a(){
return 3;
}
}

- and since the VO for `a` does not access any identifiers in the
scope chain, and since its scope is known, it can be inlined, where it
appears within that shared scope.

function x(){
return 3;
}

Such optimizations are possible iff a scope tree is built.

That's a lot of explanation and a lot for you to read. I hope it made sense.

>>  * confuses FunctionExpression and FunctionDeclaration (now that's alarming)
>
> this was a small bug, and was fixed a long time ago.
>

Good to know.

>>  * removes conditional comments (another topic, but still a problem).
>
> i don't know of any JS compressor that preserves conditional comments
> correctly in all situations. If you can name one, then I'd love to see
> it.
>

I have used JScript conditional comments only in one place, and only
to identify older version of IE to do an event handler purge onunload
(doesn't break bfcache in good browsers) Checking two ScriptEngine
properties could work, e.g.

var scriptEngineVersion = +(ScriptEngineMajorVersion() + "."
+ScriptEngineMinorVersion());

- and it's not awful, but I did not want to have to change my code to
make closure compiler happy.

[...]

Chad Killingsworth

unread,
Apr 25, 2010, 8:28:05 PM4/25/10
to Closure Compiler Discuss
In your example:

function x(){
var ChocolateFugdeSundae = 3,
crap = 7;
return a;
function a(){
return b();
}
function b(){
return;
}
}

Not only is the function returned by "x" never called, but "x" itself
is never called. Thus the entire function declaration is dead code and
is correctly removed. See http://code.google.com/closure/compiler/docs/api-tutorial3.html
The error message itself told you exactly what would prevent this
action. Exporting the function like so:

function x(){
var ChocolateFugdeSundae = 3,
crap = 7;
return a;
function a(){
return b();
}
function b(){
return;
}
}
window["x"] = x;

Compiles to:

function a() {
function b() { }
return b
}
window.x = a;

> - and it's not awful, but I did not want to have to change my code to
> make closure compiler happy.

If you don't want to change or annotate your code, than Closure-
Compiler with advanced optimizations is the wrong compressor for you.

Chad Killingsworth

Garrett Smith

unread,
Apr 25, 2010, 8:49:21 PM4/25/10
to closure-comp...@googlegroups.com
On Sun, Apr 25, 2010 at 5:28 PM, Chad Killingsworth
<chadkill...@missouristate.edu> wrote:
> In your example:
>
> function x(){
>  var ChocolateFugdeSundae = 3,
>  crap = 7;
>  return a;
>  function a(){
>    return b();
>  }
>  function b(){
>    return;
>  }
> }
>
> Not only is the function returned by "x" never called, but "x" itself
> is never called.


That is a presumptous and false statement. There is no knowing if `x`
is called elsewhere. In fact, it was called, and if you had read the
entire explanation which followed that, you would have seen that it
was called and that calling it resulted in a TypeError.

Thus the entire function declaration is dead code and
> is correctly removed. See http://code.google.com/closure/compiler/docs/api-tutorial3.html

It does what it is designed to do, and that could be greatly improved upon.

> The error message itself told you exactly what would prevent this
> action. Exporting the function like so:
>
> function x(){
>  var ChocolateFugdeSundae = 3,
>  crap = 7;
>  return a;
>  function a(){
>    return b();
>  }
>  function b(){
>    return;
>  }
> }
> window["x"] = x;
>

That last statement adds clutter. It creates a coupling of the code to
the tool. There is no apparent reason why it is there. I would hope
that if I submitted code like that for a review, it would get flagged
right away.

Chad Killingsworth

unread,
Apr 25, 2010, 9:28:01 PM4/25/10
to Closure Compiler Discuss
> That is a presumptous and false statement. There is no knowing if `x`
> is called elsewhere. In fact, it was called, and if you had read the
> entire explanation which followed that, you would have seen that it
> was called and that calling it resulted in a TypeError.

Sure there is - by definition it's not allowed to be. A stated
limitation of Closure-Compiler using advanced optimizations is that
you cannot call a function outside of the compiled code unless you
export the symbol. It sounds like this is not the behavior you desire.

> That last statement adds clutter. It creates a coupling of the code to
> the tool. There is no apparent reason why it is there. I would hope
> that if I submitted code like that for a review, it would get flagged
> right away.

I would agree that it adds a very slight amount of clutter. I've
argued that myself in certain situations. However I disagree that it
couples the code. The code will still execute correctly without being
compiled and introduces no new dependencies.

You seem to be expecting Closure-Compiler to be a general,
conservative, code-safe compressor. It is not (nor do I believe it was
ever intended to be).

Chad Killingsworth

Garrett Smith

unread,
Apr 25, 2010, 11:10:13 PM4/25/10
to closure-comp...@googlegroups.com
On Sun, Apr 25, 2010 at 6:28 PM, Chad Killingsworth
<chadkill...@missouristate.edu> wrote:
>> That is a presumptous and false statement. There is no knowing if `x`
>> is called elsewhere. In fact, it was called, and if you had read the
>> entire explanation which followed that, you would have seen that it
>> was called and that calling it resulted in a TypeError.
>
> Sure there is - by definition it's not allowed to be. A stated
> limitation of Closure-Compiler using advanced optimizations is that
> you cannot call a function outside of the compiled code unless you
> export the symbol. It sounds like this is not the behavior you desire.
>
>> That last statement adds clutter. It creates a coupling of the code to
>> the tool. There is no apparent reason why it is there. I would hope
>> that if I submitted code like that for a review, it would get flagged
>> right away.
>
> I would agree that it adds a very slight amount of clutter. I've
> argued that myself in certain situations. However I disagree that it
> couples the code. The code will still execute correctly without being
> compiled and introduces no new dependencies.
>

It couples redundant code to the tool. Dead and useless code and
should be removed. If closure compiler is replaced or removed, the
code will still have the useless:-

window["x"] = x;

- at that point, the statement seems to have absolutely no
justification whatsoever for being there. It becomes the task of the
person who is maintaining that code to identify why it is there and if
he cannot, then to remove that code.

The alternative would be to put an equally useless comment above the
statement. e.g.

// Make Google Closure Compiler happy by assigning
// a reference to x to window..
window["x"] = x;

The problem with that is that the comment may become outdated. This
can very easily happen, for example, if x is renamed to y, and added
to something else as a property:

// Make Google Closure Compiler happy by assigning
// a reference to x to window.
Spoon["y"] = y;

References to x can be updated to Spoon.y The code will work and
Closure Compiler will be happy, but the comment won't make much sense.
To the person maintaining that code, it will be unclear what the
comment means and why that useless code is there. Realizing that "it
works", the maintainer may likely leave it until something breaks.

In a 10 LOC project such as `x`, code that needs to be changed is easy
to spot. However in a 10k SLOC project, the changes are going to be
all over the place.

> You seem to be expecting Closure-Compiler to be a general,
> conservative, code-safe compressor. It is not (nor do I believe it was
> ever intended to be).
>

No, I am not expecting Closure-Compiler to be a general, conservative
code-safe compressor.

I evaluated it, found that it made some aggressive optimizations, but
that it seems to have room for improvement. I explained and how and
why and provided detailed feedback of those reasons, as requested.

When as scope tree is built, there will be no need to for such export
statements in the source code. Anything in global scope is at the root
scope tree and so cannot be removed. A scope tree allows for more
aggressive, yet safer optimizations. Such optimizations are not being
made, as stated and shown, in dead branches and also as stated and
shown with function inlining. One more example of where function
inlining would be possible:

Garrett

Nick Santos

unread,
Apr 26, 2010, 10:02:05 AM4/26/10
to closure-comp...@googlegroups.com
On Sat, Apr 24, 2010 at 4:23 AM, Klaus Reimer <k...@ailis.de> wrote:
> Alan Leung wrote:
>> By unused code, did you mean unreachable code or unused symbols?
>
> I mean unused symbols like private fields, methods and functions and
> local variables.

To speak to Klaus' original question:

There are a couple of different classes of checks here that are interesting.

- Unreachable branches: Closure-compiler does ok at this today. I
think Alan made a bunch of changes in the last week that should make
this detection a lot better.

- Dead assignments: I vaguely remember Alan writing one, but I can't
find it right now, so maybe I'm imagining things. This would be
straightforward to write, given existing infrastructure for analyzing
dead assigns.

- Unused private fields/methods: There's nothing like this today. This
would be tricky to do for code without good type annotations, but
shouldn't be too hard for code with them.

Nick

Klaus Reimer

unread,
Apr 26, 2010, 10:14:20 AM4/26/10
to closure-comp...@googlegroups.com
Nick Santos wrote:
> - Unused private fields/methods: There's nothing like this today. This
> would be tricky to do for code without good type annotations, but
> shouldn't be too hard for code with them.

Would be nice enough to have it for annotated code. At least for me.
Sorry if this sounds egoistic ;-)

I don't use the closure compiler as it is meant to be used. I don't use
the closure library (no provide/require stuff) and I disabled all
renaming and removing because I'm mostly using it to compile libraries
which may be used WITHOUT the closure compiler and especially the
renaming features are not suited for this scenario. So I'm using the
compiler only for simple compression and for heavy validation. I've
enabled all checks I could find in the configuration class. All of my
code is fully annotated so for me it is not a problem when the "unused
symbol" check only works with correctly annotated private symbols (and
local variables)

--
Bye, K <http://www.ailis.de/~k/>
[A735 47EC D87B 1F15 C1E9 53D3 AA03 6173 A723 E391]
(Finger k...@ailis.de to get public key)


Nick Santos

unread,
Apr 26, 2010, 10:18:38 AM4/26/10
to closure-comp...@googlegroups.com
On Sun, Apr 25, 2010 at 11:10 PM, Garrett Smith <dhtmlk...@gmail.com> wrote:
> When as scope tree is built, there will be no need to for such export
> statements in the source code. Anything in global scope is at the root
> scope tree and so cannot be removed. A scope tree allows for more
> aggressive, yet safer optimizations. Such optimizations are not being
> made, as stated and shown, in dead branches and also as stated and
> shown with function inlining.

Garrett,

Obviously, closure-compiler builds scope information. Otherwise, how
would it do advanced optimizations?
http://code.google.com/p/closure-compiler/source/browse/trunk/src/com/google/javascript/jscomp/SyntacticScopeCreator.java

Historically, most of the compiler plugins are written to work in
ADVANCED mode, because that's the mode that most of our apps use it in
most of the time. Right now, there's a plugin that removes all unused
variables (even global ones), but not one that just removes local
ones. It would be easy to derive the latter from the former. Thanks
for letting us know that people want this!

One minor style note--most people i know separate the exports from the
rest of their code. In your case, "window['x'] = x;" would be in a
separate file dedicated to exporting symbols.

Nick

John Lenz

unread,
Apr 26, 2010, 10:58:46 AM4/26/10
to closure-comp...@googlegroups.com
Actually, Alan enabled local unused var removal at the end of Oct.   With the local function inlining enabled in SIMPLE mode (enabled on 4/16), SIMPLE has most of the local optimization that ADVANCED mode performs: variable inlining, function inlining, unused var removal, dead assignment elimination, variable coalescing, constant folding (and code shaping), dead code removal and label renaming.

I would like to reiterate that SIMPLE mode does not modify global names and sounds exactly like what Garrett is looking for, which is great, because we already have it!

Alan Leung

unread,
Apr 26, 2010, 2:05:21 PM4/26/10
to closure-comp...@googlegroups.com

- Unreachable branches: Closure-compiler does ok at this today. I
think Alan made a bunch of changes in the last week that should make
this detection a lot better.

- Dead assignments: I vaguely remember Alan writing one, but I can't
find it right now, so maybe I'm imagining things. This would be
straightforward to write, given existing infrastructure for analyzing
dead assigns.


FYI: You are not imagining things but there are no checked in code to warn you about dead assignments. Instead, I ran the pass on closure library and looked at the logs, removed the assignments and checked in as changes. It will be very straight forward to add that as a warning so every code base can benefit from it.

Garrett Smith

unread,
Apr 27, 2010, 3:16:55 AM4/27/10
to closure-comp...@googlegroups.com
On Mon, Apr 26, 2010 at 7:18 AM, Nick Santos <nicks...@google.com> wrote:
> On Sun, Apr 25, 2010 at 11:10 PM, Garrett Smith <dhtmlk...@gmail.com> wrote:
>> When as scope tree is built, there will be no need to for such export
>> statements in the source code. Anything in global scope is at the root
>> scope tree and so cannot be removed. A scope tree allows for more
>> aggressive, yet safer optimizations. Such optimizations are not being
>> made, as stated and shown, in dead branches and also as stated and
>> shown with function inlining.
>
> Garrett,
>
> Obviously, closure-compiler builds scope information. Otherwise, how
> would it do advanced optimizations?

No idea. The advanced optimizations don't seem to follow a sensible strategy.

> http://code.google.com/p/closure-compiler/source/browse/trunk/src/com/google/javascript/jscomp/SyntacticScopeCreator.java
>

I'm looking at `scanRoot`
http://code.google.com/p/closure-compiler/source/browse/trunk/src/com/google/javascript/jscomp/SyntacticScopeCreator.java#84

I see that a function declaration is added as an identifier to the
parent scope, if it does not already exist.
http://code.google.com/p/closure-compiler/source/browse/trunk/src/com/google/javascript/jscomp/SyntacticScopeCreator.java#92

Am I reading that wrong, or is that code stepping outside of what is
specified in ECMA-262 and adding that identifier to the containing
scope?

interface ScopeCreator seems to not know what global scope is.

/**
* Creates a {@link Scope} object.
*
* @param n the root node (either a FUNCTION node, a SCRIPT node, or a
* synthetic block node whose children are all SCRIPT nodes)
* @param parent the parent Scope object (may be null)
*/

http://www.google.com/codesearch/p?hl=en#l5BkQmivP-Y/trunk/src/com/google/javascript/jscomp/ScopeCreator.java&q=scope%20package:http://closure-compiler\.googlecode\.com&d=6

> Historically, most of the compiler plugins are written to work in
> ADVANCED mode, because that's the mode that most of our apps use it in
> most of the time. Right now, there's a plugin that removes all unused
> variables (even global ones), but not one that just removes local
> ones. It would be easy to derive the latter from the former. Thanks
> for letting us know that people want this!
>

Many of the google applications use invalid ECMAScript, which is error
corrected using different techniques in implementations. Namely
FunctionDeclaration appearing in place where only statement is
allowed. Is this being described by the nonstandard terminology in the
comment above: "a synthetic block node" ?

[...]


Was the explanation of scope tree not clear enough?

Nick Santos

unread,
Apr 27, 2010, 8:27:19 AM4/27/10
to closure-comp...@googlegroups.com
On Tue, Apr 27, 2010 at 3:16 AM, Garrett Smith <dhtmlk...@gmail.com> wrote:
>> Obviously, closure-compiler builds scope information. Otherwise, how
>> would it do advanced optimizations?
>
> No idea. The advanced optimizations don't seem to follow a sensible strategy.

you got us! advanced mode is just implemented as (Math.random() > 0.5
? removeCode() : leaveCode());

> I see that a function declaration is added as an identifier to the
> parent scope, if it does not already exist.
> http://code.google.com/p/closure-compiler/source/browse/trunk/src/com/google/javascript/jscomp/SyntacticScopeCreator.java#92
>
> Am I reading that wrong, or is that code stepping outside of what is
> specified in ECMA-262 and adding that identifier to the containing
> scope?

Did you read the comment?
// Bleed the function name into the scope, if it hasn't
// been declared in the outer scope.
This means that if you have:
var factorial = function y(x) { return x ? x * y(x - 1) : 1; };
Then, yes, "y" is added as a symbol to the inner scope and not the outer scope.

> Many of the google applications use invalid ECMAScript, which is error
> corrected using different techniques in implementations. Namely
> FunctionDeclaration appearing in place where only statement is
> allowed.

I assume your comment is referring to the phenomenon described by
section 2.9 of this doc:
http://wiki.ecmascript.org/lib/exe/fetch.php?id=resources%3Aresources&cache=cache&media=resources:jscriptdeviationsfromes3.pdf

This brings up a fun philosophical question: if a specification author
says something in the forest, and no browser actually implements it,
does he make a sound?

Nick

Mike Samuel

unread,
Apr 27, 2010, 1:56:30 PM4/27/10
to closure-comp...@googlegroups.com
2010/4/27 Nick Santos <nicks...@google.com>:
The EcmaScript 5 committee wanted to define the semantics of it but
put it off because they wanted to do it in terms of let-scoping, and
waiting for let-scoping to be hashed out would have delayed ES5 for
too long.
It is very likely that ES harmony will nail this down.

Garrett Smith

unread,
Apr 27, 2010, 2:55:22 PM4/27/10
to closure-comp...@googlegroups.com
On Tue, Apr 27, 2010 at 5:27 AM, Nick Santos <nicks...@google.com> wrote:
> On Tue, Apr 27, 2010 at 3:16 AM, Garrett Smith <dhtmlk...@gmail.com> wrote:
>>> Obviously, closure-compiler builds scope information. Otherwise, how
>>> would it do advanced optimizations?
>>
>> No idea. The advanced optimizations don't seem to follow a sensible strategy.
>
> you got us! advanced mode is just implemented as (Math.random() > 0.5
> ? removeCode() : leaveCode());
>

That is a nonsensical, but random strategy.

You have disregarded the long and careful explanation of scope tree
that I provided and responded with what appears to be arrogance and
sarcasm.

>> I see that a function declaration is added as an identifier to the
>> parent scope, if it does not already exist.
>> http://code.google.com/p/closure-compiler/source/browse/trunk/src/com/google/javascript/jscomp/SyntacticScopeCreator.java#92
>>
>> Am I reading that wrong, or is that code stepping outside of what is
>> specified in ECMA-262 and adding that identifier to the containing
>> scope?
>
> Did you read the comment?
>      // Bleed the function name into the scope, if it hasn't
>      // been declared in the outer scope.
> This means that if you have:
> var factorial = function y(x) { return x ? x * y(x - 1) : 1;  };
> Then, yes, "y" is added as a symbol to the inner scope and not the outer scope.
>
>> Many of the google applications use invalid ECMAScript, which is error
>> corrected using different techniques in implementations. Namely
>> FunctionDeclaration appearing in place where only statement is
>> allowed.
>
> I assume your comment is referring to the phenomenon described by
> section 2.9 of this doc:
> http://wiki.ecmascript.org/lib/exe/fetch.php?id=resources%3Aresources&cache=cache&media=resources:jscriptdeviationsfromes3.pdf
>

No. What is written there is a false statement:

| When a function is defined using a function declaration,
| JScript binds the function to the global scope regardless
| of any scope changes introduced by with clauses.

.

Completely false statement and even if it were true, what is written
in the java source file does not not what is stated there.

> This brings up a fun philosophical question: if a specification author
> says something in the forest, and no browser actually implements it,
> does he make a sound?
>

I have carefully explained scope. I have described, with examples, how
to build a scope tree.

I will be providing no further assistance to you, except to point you
to the correct, updated JScript Deviations document that you cited
earlier.

The correct, updated version is linked from the FAQ, along with the
official ECMAScript specifications for editions 3 and 5, as well as
ISO/IEC 16262.

http://jibbering.com/faq/#ecmaResources

Nick Santos

unread,
Apr 27, 2010, 3:18:23 PM4/27/10
to closure-comp...@googlegroups.com
The bug you described earlier has been fixed in revision 199. (John
filed it in issue 155)
http://code.google.com/p/closure-compiler/issues/detail?id=155

Thanks for the report!
Reply all
Reply to author
Forward
0 new messages