Disallow continue referring to a non-IterationStatement. Fix #287

Simplifies handling of labels (their definition/references can be easily
figured out at parse time, no need to do it in `figure_out_scope`).
This commit is contained in:
Mihai Bazon
2013-09-02 19:36:16 +03:00
parent 1c6efdae34
commit 85b527ba3d
4 changed files with 34 additions and 48 deletions

View File

@@ -197,6 +197,10 @@ var AST_LabeledStatement = DEFNODE("LabeledStatement", "label", {
}
}, AST_StatementWithBody);
var AST_IterationStatement = DEFNODE("Loop", null, {
$documentation: "Internal class. All loops inherit from it."
}, AST_StatementWithBody);
var AST_DWLoop = DEFNODE("DWLoop", "condition", {
$documentation: "Base class for do/while statements",
$propdoc: {
@@ -208,7 +212,7 @@ var AST_DWLoop = DEFNODE("DWLoop", "condition", {
this.body._walk(visitor);
});
}
}, AST_StatementWithBody);
}, AST_IterationStatement);
var AST_Do = DEFNODE("Do", null, {
$documentation: "A `do` statement",
@@ -233,7 +237,7 @@ var AST_For = DEFNODE("For", "init condition step", {
this.body._walk(visitor);
});
}
}, AST_StatementWithBody);
}, AST_IterationStatement);
var AST_ForIn = DEFNODE("ForIn", "init name object", {
$documentation: "A `for ... in` statement",
@@ -249,7 +253,7 @@ var AST_ForIn = DEFNODE("ForIn", "init name object", {
this.body._walk(visitor);
});
}
}, AST_StatementWithBody);
}, AST_IterationStatement);
var AST_With = DEFNODE("With", "expression", {
$documentation: "A `with` statement",
@@ -821,7 +825,11 @@ var AST_SymbolCatch = DEFNODE("SymbolCatch", null, {
var AST_Label = DEFNODE("Label", "references", {
$documentation: "Symbol naming a label (declaration)",
$propdoc: {
references: "[AST_LabelRef*] a list of nodes referring to this label"
references: "[AST_LoopControl*] a list of nodes referring to this label"
},
initialize: function() {
this.references = [];
this.thedef = this;
}
}, AST_Symbol);

View File

@@ -324,7 +324,7 @@ merge(Compressor.prototype, {
|| (ab instanceof AST_Continue && self === loop_body(lct))
|| (ab instanceof AST_Break && lct instanceof AST_BlockStatement && self === lct))) {
if (ab.label) {
remove(ab.label.thedef.references, ab.label);
remove(ab.label.thedef.references, ab);
}
CHANGED = true;
var body = as_statement_array(stat.body).slice(0, -1);
@@ -346,7 +346,7 @@ merge(Compressor.prototype, {
|| (ab instanceof AST_Continue && self === loop_body(lct))
|| (ab instanceof AST_Break && lct instanceof AST_BlockStatement && self === lct))) {
if (ab.label) {
remove(ab.label.thedef.references, ab.label);
remove(ab.label.thedef.references, ab);
}
CHANGED = true;
stat = stat.clone();
@@ -385,7 +385,7 @@ merge(Compressor.prototype, {
&& loop_body(lct) === self) || (stat instanceof AST_Continue
&& loop_body(lct) === self)) {
if (stat.label) {
remove(stat.label.thedef.references, stat.label);
remove(stat.label.thedef.references, stat);
}
} else {
a.push(stat);

View File

@@ -828,6 +828,18 @@ function parse($TEXT, options) {
S.labels.push(label);
var stat = statement();
S.labels.pop();
if (!(stat instanceof AST_IterationStatement)) {
// check for `continue` that refers to this label.
// those should be reported as syntax errors.
// https://github.com/mishoo/UglifyJS2/issues/287
label.references.forEach(function(ref){
if (ref instanceof AST_Continue) {
ref = ref.label.start;
croak("Continue label `" + label.name + "` refers to non-loop statement.",
ref.line, ref.col, ref.pos);
}
});
}
return new AST_LabeledStatement({ body: stat, label: label });
};
@@ -836,18 +848,22 @@ function parse($TEXT, options) {
};
function break_cont(type) {
var label = null;
var label = null, ldef;
if (!can_insert_semicolon()) {
label = as_symbol(AST_LabelRef, true);
}
if (label != null) {
if (!find_if(function(l){ return l.name == label.name }, S.labels))
ldef = find_if(function(l){ return l.name == label.name }, S.labels);
if (!ldef)
croak("Undefined label " + label.name);
label.thedef = ldef;
}
else if (S.in_loop == 0)
croak(type.TYPE + " not inside a loop or switch");
semicolon();
return new type({ label: label });
var stat = new type({ label: label });
if (ldef) ldef.references.push(stat);
return stat;
};
function for_() {

View File

@@ -82,18 +82,14 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(){
// pass 1: setup scope chaining and handle definitions
var self = this;
var scope = self.parent_scope = null;
var labels = new Dictionary();
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;
var save_labels = labels;
++nesting;
scope = node;
labels = new Dictionary();
descend();
labels = save_labels;
scope = save_scope;
--nesting;
return true; // don't descend again in TreeWalker
@@ -108,22 +104,9 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(){
s.uses_with = true;
return;
}
if (node instanceof AST_LabeledStatement) {
var l = node.label;
if (labels.has(l.name))
throw new Error(string_template("Label {name} defined twice", l));
labels.set(l.name, l);
descend();
labels.del(l.name);
return true; // no descend again
}
if (node instanceof AST_Symbol) {
node.scope = scope;
}
if (node instanceof AST_Label) {
node.thedef = node;
node.init_scope_vars();
}
if (node instanceof AST_SymbolLambda) {
scope.def_function(node);
}
@@ -150,15 +133,6 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(){
// identifier in the enclosing scope)
scope.def_variable(node);
}
if (node instanceof AST_LabelRef) {
var sym = labels.get(node.name);
if (!sym) throw new Error(string_template("Undefined label {name} [{line},{col}]", {
name: node.name,
line: node.start.line,
col: node.start.col
}));
node.thedef = sym;
}
});
self.walk(tw);
@@ -173,10 +147,6 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(){
func = prev_func;
return true;
}
if (node instanceof AST_LabelRef) {
node.reference();
return true;
}
if (node instanceof AST_SymbolRef) {
var name = node.name;
var sym = node.scope.find_variable(name);
@@ -241,14 +211,6 @@ AST_SymbolRef.DEFMETHOD("reference", function() {
this.frame = this.scope.nesting - def.scope.nesting;
});
AST_Label.DEFMETHOD("init_scope_vars", function(){
this.references = [];
});
AST_LabelRef.DEFMETHOD("reference", function(){
this.thedef.references.push(this);
});
AST_Scope.DEFMETHOD("find_variable", function(name){
if (name instanceof AST_Symbol) name = name.name;
return this.variables.get(name)