Skip to content

Commit

Permalink
Fix duplicate vars at top level
Browse files Browse the repository at this point in the history
To see the problem this patch fixes, look at the first line of
lib/cli.js:

```js
var $0, $0, ...
```

The Program node handler was attempting to deduplicate with `nub`, but
doing so over a list of JS.Identifiers, not a list of plain strings.

Since most callers of `declarationsNeededRecursive` are more interested
in the set of names than the identifier nodes, I changed it to do that.
  • Loading branch information
ef4 committed May 28, 2015
1 parent 7a2eabe commit b0785ca
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 22 deletions.
2 changes: 1 addition & 1 deletion lib/cli.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 9 additions & 13 deletions lib/compiler.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/js-nodes.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/parser.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions src/compiler.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ generateMutatingWalker = (fn) -> (node, args...) ->

declarationsNeeded = (node) ->
return [] unless node?
if (node.instanceof JS.AssignmentExpression) and node.operator is '=' and node.left.instanceof JS.Identifier then [node.left]
else if (node.instanceof JS.ForInStatement) and node.left.instanceof JS.Identifier then [node.left]
if (node.instanceof JS.AssignmentExpression) and node.operator is '=' and node.left.instanceof JS.Identifier then [node.left.name]
else if (node.instanceof JS.ForInStatement) and node.left.instanceof JS.Identifier then [node.left.name]
else []

declarationsNeededRecursive = (node) ->
Expand Down Expand Up @@ -415,7 +415,7 @@ class exports.Compiler
decls = nub concatMap block, declarationsNeededRecursive
if decls.length > 0
if options.bare
block.unshift makeVarDeclaration decls
block.unshift makeVarDeclaration decls.map (name) -> new JS.Identifier(name)
else
# add a function wrapper
block = [stmt new JS.UnaryExpression 'void', new JS.CallExpression (memberAccess (new JS.FunctionExpression null, [], new JS.BlockStatement block), 'call'), [new JS.ThisExpression]]
Expand Down Expand Up @@ -1095,7 +1095,7 @@ class exports.Compiler

# TODO: comments
generateMutatingWalker (state) ->
state.declaredSymbols = union state.declaredSymbols, map (declarationsNeeded this), (id) -> id.name
state.declaredSymbols = union state.declaredSymbols, declarationsNeeded this
{declaredSymbols, usedSymbols, nsCounters} = state
newNode = if @instanceof JS.GenSym
newNode = new JS.Identifier generateName this, state
Expand All @@ -1110,15 +1110,15 @@ class exports.Compiler
usedSymbols: union usedSymbols, params
nsCounters: nsCounters_
newNode.body = forceBlock newNode.body
undeclared = map (declarationsNeededRecursive @body), (id) -> id.name
undeclared = declarationsNeededRecursive @body
undeclared = difference undeclared, map (variableDeclarations @body), (id) -> id.name
alreadyDeclared = union declaredSymbols, concatMap @params, collectIdentifiers
declNames = nub difference undeclared, alreadyDeclared
decls = map declNames, (name) -> new JS.Identifier name
newNode.body.body.unshift makeVarDeclaration decls if decls.length > 0
newNode
else generateSymbols this, state
state.declaredSymbols = union declaredSymbols, map (declarationsNeededRecursive newNode), (id) -> id.name
state.declaredSymbols = union declaredSymbols, declarationsNeededRecursive newNode
newNode

defaultRule = ->
Expand Down

0 comments on commit b0785ca

Please sign in to comment.