From 85b527ba3d609e87b5b5f758b429ef371dc3e459 Mon Sep 17 00:00:00 2001 From: Mihai Bazon Date: Mon, 2 Sep 2013 19:36:16 +0300 Subject: [PATCH] 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`). --- lib/ast.js | 16 ++++++++++++---- lib/compress.js | 6 +++--- lib/parse.js | 22 +++++++++++++++++++--- lib/scope.js | 38 -------------------------------------- 4 files changed, 34 insertions(+), 48 deletions(-) diff --git a/lib/ast.js b/lib/ast.js index b8e89f23..e51cdd02 100644 --- a/lib/ast.js +++ b/lib/ast.js @@ -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); diff --git a/lib/compress.js b/lib/compress.js index 8d936bac..0d2053e7 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -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); diff --git a/lib/parse.js b/lib/parse.js index abd4d7b7..6946d93c 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -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_() { diff --git a/lib/scope.js b/lib/scope.js index d15cec75..8fc1ce0b 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -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) -- 2.34.1