http://codereview.appspot.com/110096/diff/2030/3041
File src/com/google/caja/parser/quasiliteral/AlphaRenaming.java (right):
http://codereview.appspot.com/110096/diff/2030/3041#newcode62
Line 62: * not to conflict with generated code names.
On 2009/09/14 20:13:18, ihab.awad wrote:
Perhaps explain why this class takes an Expression and not something
else? Can
just cut & paste from your review comments to me? ;)
Done.
http://codereview.appspot.com/110096/diff/2030/3041#newcode132
Line 132: Expression e, NameContext<String, ?> ctxt, Set<String>
freeSynthetics,
On 2009/09/14 20:13:18, ihab.awad wrote:
So you don't think it's a good idea to rename "ctxt" and
"freeSynthetics"
(whatever we end up calling them)?
Done.
http://codereview.appspot.com/110096/diff/2030/3040
File src/com/google/caja/parser/quasiliteral/AlphaRenamingRewriter.java
(right):
http://codereview.appspot.com/110096/diff/2030/3040#newcode60
Line 60: matches="/* Reference */ @r",
On 2009/09/14 20:13:18, ihab.awad wrote:
/* Expression */ ??
Done.
http://codereview.appspot.com/110096/diff/2030/3036
File tests/com/google/caja/parser/quasiliteral/AlphaRenamingTest.java
(right):
http://codereview.appspot.com/110096/diff/2030/3036#newcode63
Line 63: public final void testPropertNames() throws Exception {
On 2009/09/14 20:13:18, ihab.awad wrote:
testPropert*y*Names
Done.
http://codereview.appspot.com/110096/diff/2030/3036#newcode104
Line 104: );
On 2009/09/14 20:13:18, ihab.awad wrote:
Move closing paren 1 line up?
Done.
http://codereview.appspot.com/110096/diff/2030/3036#newcode113
Line 113: + " var c = b;"
On 2009/09/14 20:13:18, ihab.awad wrote:
Fwiw, the reason for this rewriting -- the addition of the function
name as a
"var" -- was not clear to me from reading the code. I suspect it has
to do with
trying to regularize the underlying semantics of function name scoping
in JS ...
but how precisely? By capturing the reference to the enclosing
function even if
that symbol is assigned to later on in the containing scope?
Exactly.
I'm trying to recognize that the name used to refer to the function
inside it's body is effectively a different declaration than the one
introduced in its parent scope.
Since they're different they have exactly the behavior you describe.
http://codereview.appspot.com/110096/diff/2030/3036#newcode178
Line 178: + " function c() {"
I think you identified a bug. The declaration
var c = b
should not be there since it clobbers the hoisted local declaration
function c().
In squarefree,
function f() { var f; return f; }
'undefined' === typeof f()
On 2009/09/14 20:13:18, ihab.awad wrote:
What is the rationale for not coming up with *yet* another variable
here, so
that it would be --
(function () {
function b() {
var c = b;
function d() {
var e = d;
return e, a;
}
}
return b;
})
?
http://codereview.appspot.com/110096/diff/2030/3036#newcode233
Line 233: + " function c(e) { var d = c; return e * e; }"
On 2009/09/14 20:13:18, ihab.awad wrote:
We should have a TODO for optimizing out the function name assignment
"var" if
that name is unused. It would be too messy to do now but maybe, in a
future
society, we would have a scope representation that would allow that to
be done
simply.
Done.
http://codereview.appspot.com/110096
