Properly scope catch identifier when --screw-ie8

Fix #344
This commit is contained in:
Mihai Bazon
2013-11-28 16:43:30 +02:00
parent ea10642572
commit d2190c2bf3
3 changed files with 19 additions and 29 deletions

View File

@@ -498,12 +498,6 @@ var AST_Try = DEFNODE("Try", "bcatch bfinally", {
}
}, AST_Block);
// XXX: this is wrong according to ECMA-262 (12.4). the catch block
// should introduce another scope, as the argname should be visible
// only inside the catch block. However, doing it this way because of
// IE which simply introduces the name in the surrounding scope. If
// we ever want to fix this then AST_Catch should inherit from
// AST_Scope.
var AST_Catch = DEFNODE("Catch", "argname", {
$documentation: "A `catch` node; only makes sense as part of a `try` statement",
$propdoc: {
@@ -515,7 +509,7 @@ var AST_Catch = DEFNODE("Catch", "argname", {
walk_body(this, visitor);
});
}
}, AST_Block);
}, AST_Scope);
var AST_Finally = DEFNODE("Finally", null, {
$documentation: "A `finally` node; only makes sense as part of a `try` statement"

View File

@@ -1669,10 +1669,10 @@ merge(Compressor.prototype, {
return arg.value;
}).join(",") + "){" + self.args[self.args.length - 1].value + "})()";
var ast = parse(code);
ast.figure_out_scope();
ast.figure_out_scope({ screw_ie8: compressor.option("screw_ie8") });
var comp = new Compressor(compressor.options);
ast = ast.transform(comp);
ast.figure_out_scope();
ast.figure_out_scope({ screw_ie8: compressor.option("screw_ie8") });
ast.mangle_names();
var fun;
try {

View File

@@ -71,27 +71,28 @@ SymbolDef.prototype = {
}
};
AST_Toplevel.DEFMETHOD("figure_out_scope", function(){
// This does what ast_add_scope did in UglifyJS v1.
//
// Part of it could be done at parse time, but it would complicate
// the parser (and it's already kinda complex). It's also worth
// having it separated because we might need to call it multiple
// times on the same tree.
AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){
options = defaults(options, {
screw_ie8: false
});
// pass 1: setup scope chaining and handle definitions
var self = this;
var scope = self.parent_scope = null;
var defun = null;
var nesting = 0;
var tw = new TreeWalker(function(node, descend){
if (node instanceof AST_Scope) {
node.init_scope_vars(nesting);
var save_scope = node.parent_scope = scope;
++nesting;
var save_defun = defun;
if (!(node instanceof AST_Catch)) {
defun = node;
}
scope = node;
descend();
++nesting; descend(); --nesting;
scope = save_scope;
--nesting;
defun = save_defun;
return true; // don't descend again in TreeWalker
}
if (node instanceof AST_Directive) {
@@ -108,7 +109,7 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(){
node.scope = scope;
}
if (node instanceof AST_SymbolLambda) {
scope.def_function(node);
defun.def_function(node);
}
else if (node instanceof AST_SymbolDefun) {
// Careful here, the scope where this should be defined is
@@ -116,22 +117,17 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(){
// scope when we encounter the AST_Defun node (which is
// instanceof AST_Scope) but we get to the symbol a bit
// later.
(node.scope = scope.parent_scope).def_function(node);
(node.scope = defun.parent_scope).def_function(node);
}
else if (node instanceof AST_SymbolVar
|| node instanceof AST_SymbolConst) {
var def = scope.def_variable(node);
var def = defun.def_variable(node);
def.constant = node instanceof AST_SymbolConst;
def.init = tw.parent().value;
}
else if (node instanceof AST_SymbolCatch) {
// XXX: this is wrong according to ECMA-262 (12.4). the
// `catch` argument name should be visible only inside the
// catch block. For a quick fix AST_Catch should inherit
// from AST_Scope. Keeping it this way because of IE,
// which doesn't obey the standard. (it introduces the
// identifier in the enclosing scope)
scope.def_variable(node);
(options.screw_ie8 ? scope : defun)
.def_variable(node);
}
});
self.walk(tw);