handle labels properly

(they can't be handled the same way as variables in a scope)

2 files changed, 43 insertions, 41 deletions

diff --git a/lib/ast.js b/lib/ast.js
index 89a8237b..c38b3277 100644
--- a/lib/ast.js
+++ b/lib/ast.js
@@ -158,15 +158,13 @@ var AST_With = DEFNODE("With", "expression", {
 var AST_Scope = DEFNODE("Scope", null, {
     $documentation: "Base class for all statements introducing a lexical scope",
     initialize: function() {
-        this.labels = {};         // map name to AST_Label (labels defined in this scope)
         this.variables = {};      // map name to AST_SymbolVar (variables defined in this scope; includes functions)
         this.functions = {};      // map name to AST_SymbolDefun (functions defined in this scope)
         this.uses_with = false;   // will be set to true if this or some nested scope uses the `with` statement
         this.uses_eval = false;   // will be set to true if this or nested scope uses the global `eval`
         this.parent_scope = null; // the parent scope
-        this.enclosed = [];       // a list of variables this or from outer scope(s) that are accessed from this or inner scopes
+        this.enclosed = [];       // a list of variables from this or outer scope(s) that are referenced from this or inner scopes
         this.cname = -1;          // the current index for mangling functions/variables
-        this.lname = -1;          // the current index for mangling labels
     }
 }, AST_BlockStatement);
 
@@ -568,7 +566,9 @@ function TreeWalker(callback) {
 TreeWalker.prototype = {
     _visit: function(node, descend) {
         this.stack.push(node);
-        var ret = this.visit(node, descend);
+        var ret = this.visit(node, function(){
+            descend.call(node);
+        });
         if (!ret && descend) {
             descend.call(node);
         }
diff --git a/lib/scope.js b/lib/scope.js
index 00bdcb08..a5719bf3 100644
--- a/lib/scope.js
+++ b/lib/scope.js
@@ -8,11 +8,12 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(){
 
     // pass 1: setup scope chaining and handle definitions
     var scope = null;
+    var labels = {};
     var tw = new TreeWalker(function(node, descend){
         if (node instanceof AST_Scope) {
             var save_scope = node.parent_scope = scope;
             scope = node;
-            descend.call(node);
+            descend();
             scope = save_scope;
             return true;        // don't descend again in TreeWalker
         }
@@ -21,6 +22,15 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(){
                 s.uses_with = true;
             return;
         }
+        if (node instanceof AST_LabeledStatement) {
+            var l = node.label;
+            if (labels[l.name])
+                throw new Error(string_template("Label {name} defined twice", l));
+            labels[l.name] = l;
+            descend();
+            delete labels[l.name];
+            return true;        // no descend again
+        }
         if (node instanceof AST_SymbolLambda) {
             scope.def_function(node);
         }
@@ -35,9 +45,6 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(){
         else if (node instanceof AST_SymbolVar) {
             scope.def_variable(node);
         }
-        else if (node instanceof AST_Label) {
-            scope.def_label(node);
-        }
         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
@@ -51,7 +58,7 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(){
             node.scope = scope;
         }
         if (node instanceof AST_LabelRef) {
-            var sym = scope.find_label(node);
+            var sym = labels[node.name];
             if (!sym) throw new Error("Undefined label " + node.name);
             node.reference(sym);
         }
@@ -153,11 +160,6 @@ AST_Scope.DEFMETHOD("find_variable", function(name){
         (this.parent_scope && this.parent_scope.find_variable(name));
 });
 
-AST_Scope.DEFMETHOD("find_label", function(name){
-    if (name instanceof AST_Symbol) name = name.name;
-    return this.labels[name];
-});
-
 AST_Scope.DEFMETHOD("def_function", function(symbol){
     this.functions[symbol.name] = symbol;
     this.def_variable(symbol);
@@ -174,25 +176,11 @@ AST_Scope.DEFMETHOD("def_variable", function(symbol){
     symbol.scope = this;
 });
 
-AST_Scope.DEFMETHOD("def_label", function(symbol){
-    this.labels[symbol.name] = symbol;
-    symbol.scope = this;
-});
-
-AST_Scope.DEFMETHOD("next_mangled", function(for_label){
+AST_Scope.DEFMETHOD("next_mangled", function(){
     var ext = this.enclosed, n = ext.length;
     out: while (true) {
-        var m = base54(for_label
-                       ? (++this.lname)
-                       : (++this.cname));
-
+        var m = base54(++this.cname);
         if (!is_identifier(m)) continue; // skip over "do"
-
-        // labels are easy, since they can't be referenced from nested
-        // scopes.  XXX: not sure that will be the case when the `let`
-        // keyword is to be supported.
-        if (for_label) return m;
-
         // if it's for functions or variables, we must ensure that the
         // mangled name does not shadow a name from some parent scope
         // that is referenced in this or in inner scopes.
@@ -201,7 +189,6 @@ AST_Scope.DEFMETHOD("next_mangled", function(for_label){
             var name = sym.mangled_name || sym.name;
             if (m == name) continue out;
         }
-
         return m;
     }
 });
@@ -214,10 +201,14 @@ AST_SymbolDeclaration.DEFMETHOD("mangle", function(){
                || this.scope.uses_eval
                || this.scope.uses_with
                || this.mangled_name)) {
-        this.mangled_name = this.scope.next_mangled(this instanceof AST_Label);
+        this.mangled_name = this.scope.next_mangled();
     }
 });
 
+AST_Label.DEFMETHOD("mangle", function(){
+    throw new Error("Don't call this");
+});
+
 AST_SymbolDeclaration.DEFMETHOD("unreferenced", function(){
     return this.definition().references.length == 0;
 });
@@ -227,20 +218,31 @@ AST_SymbolDeclaration.DEFMETHOD("definition", function(){
 });
 
 AST_Toplevel.DEFMETHOD("mangle_names", function(){
-    var tw = new TreeWalker(function(node){
-        // We only need to mangle declarations.  Special logic wired
-        // into the code generator will display the mangled name if
-        // it's present (and for AST_SymbolRef-s it'll use the mangled
-        // name of the AST_SymbolDeclaration that it points to).
+    // We only need to mangle declaration nodes.  Special logic wired
+    // into the code generator will display the mangled name if it's
+    // present (and for AST_SymbolRef-s it'll use the mangled name of
+    // the AST_SymbolDeclaration that it points to).
+    var lname = -1;
+    var tw = new TreeWalker(function(node, descend){
+        if (node instanceof AST_LabeledStatement) {
+            // lname is incremented when we get to the AST_Label
+            var save_nesting = lname;
+            descend();
+            lname = save_nesting;
+            return true;        // don't descend again in TreeWalker
+        }
         if (node instanceof AST_Scope) {
             var a = node.variables;
             for (var i in a) if (HOP(a, i)) {
                 a[i].mangle();
             }
-            var a = node.labels;
-            for (var i in a) if (HOP(a, i)) {
-                a[i].mangle();
-            }
+            return;
+        }
+        if (node instanceof AST_Label) {
+            var name;
+            do name = base54(++lname); while (!is_identifier(name));
+            node.mangled_name = name;
+            return;
         }
     });
     this.walk(tw);