JS: clean up reused undeclared variables when scopes close

js/ast.go

@@ -258,24 +258,6 @@ func (s *Scope) Declare(decl DeclType, name []byte) (*Var, bool) {
 					// must be NoDecl so that it can't be a var declaration that has been added
 					if uv := s.Undeclared[-index-1]; 0 < uv.Uses && uv.Decl == NoDecl {
 						v = uv
-						s.Undeclared = append(s.Undeclared[:-index-1], s.Undeclared[-index:]...)
-
-						for name, indices := range s.VarIndices {
-							var updated bool
-							for i := 0; i < len(indices); i++ {
-								if indices[i] == index {
-									indices = append(indices[:i], indices[i+1:]...)
-									updated = true
-									i--
-								} else if indices[i] < index {
-									indices[i]++
-									updated = true
-								}
-							}
-							if updated {
-								s.VarIndices[name] = indices
-							}
-						}
 						break
 					}
 				}
@@ -447,6 +429,34 @@ func (s *Scope) MarkFuncArgs() {
 
 // HoistUndeclared copies all undeclared variables of the current scope to the parent scope.
 func (s *Scope) HoistUndeclared() {
+	// Remove reused declarations once the scope closes, keeping references to outer declarations.
+	if s.VarIndices != nil {
+		n := 0
+		numArgUses := s.NumArgUses
+	UndeclaredLoop:
+		for i, v := range s.Undeclared {
+			if v.Decl != NoDecl {
+				for _, index := range s.VarIndices[string(v.Data)] {
+					if 0 <= index && s.Declared[index] == v {
+						if i < numArgUses {
+							s.NumArgUses--
+						}
+						continue UndeclaredLoop
+					}
+				}
+			}
+			s.Undeclared[n] = v
+			n++
+		}
+		if n != len(s.Undeclared) {
+			s.Undeclared = s.Undeclared[:n]
+			s.buildVarIndices()
+		}
+	}
+	if s.Parent == nil {
+		return
+	}
+
 	hoist := make([]*Var, 0, len(s.Undeclared))
 Loop:
 	for j, vorig := range s.Undeclared {

js/parse.go

@@ -60,6 +60,7 @@ func Parse(r *parse.Input, o Options) (*AST, error) {
 			}
 			ast.BlockStmt.List = append(ast.BlockStmt.List, p.parseStmt(true))
 		}
+		p.exitScope(nil)
 	} else {
 		// catch shebang in first line
 		var shebang []byte
@@ -198,6 +199,7 @@ func (p *Parser) parseModule() (module BlockStmt) {
 				module.List = append(p.comments, module.List...)
 				p.comments = p.comments[:0]
 			}
+			p.exitScope(nil)
 			return
 		case ImportToken:
 			p.next()

js/parse_test.go

@@ -1017,6 +1017,7 @@ func TestParseScope(t *testing.T) {
 		{"async function a(){ b => await%5 }", "a=1//b=3", "await=2/await=2/await=2"},
 		{"let a; {let b = a}", "a=1/b=2", "/a=1"},
 		{"let a; {var b}", "a=1,b=2/", "/b=2"}, // may remove b from uses
+		{"let a; {var b,c,d,e,f,g,h,i,j,k,l,m,n,o,p; a}", "a=1,b=2,c=3,d=4,e=5,f=6,g=7,h=8,i=9,j=10,k=11,l=12,m=13,n=14,o=15,p=16/", "/b=2,c=3,d=4,e=5,f=6,g=7,h=8,i=9,j=10,k=11,l=12,m=13,n=14,o=15,p=16,a=1"},
 		{"let a; {var b = a}", "a=1,b=2/", "/b=2,a=1"},
 		{"let a; {class b{}}", "a=1/b=2/", "//"},
 		{"a = 5; var a;", "a=1", ""},
@@ -1083,6 +1084,89 @@ func TestParseScope(t *testing.T) {
 	}
 }
 
+type scopeDisjointVisitor struct {
+	t *testing.T
+}
+
+func (v scopeDisjointVisitor) Enter(n INode) IVisitor {
+	var scope *Scope
+	switch n := n.(type) {
+	case *BlockStmt:
+		scope = &n.Scope
+	case *ClassDecl:
+		scope = &n.Scope
+	case *SwitchStmt:
+		scope = &n.Scope
+	}
+	if scope != nil {
+		if scope.NumArgUses > len(scope.Undeclared) {
+			v.t.Errorf("argument uses offset %d exceeds %d undeclared variables", scope.NumArgUses, len(scope.Undeclared))
+		}
+		for name, indices := range scope.VarIndices {
+			for _, index := range indices {
+				if index < 0 {
+					if len(scope.Undeclared) <= -index-1 || string(scope.Undeclared[-index-1].Data) != name {
+						v.t.Errorf("invalid undeclared index %d for %s", index, name)
+					}
+				} else if len(scope.Declared) <= index || string(scope.Declared[index].Data) != name {
+					v.t.Errorf("invalid declared index %d for %s", index, name)
+				}
+			}
+		}
+		declared := make(map[*Var]bool, len(scope.Declared))
+		for _, variable := range scope.Declared {
+			declared[variable] = true
+		}
+		for _, variable := range scope.Undeclared {
+			for variable.Link != nil {
+				variable = variable.Link
+			}
+			if declared[variable] {
+				v.t.Errorf("%s is both declared and undeclared", variable.Data)
+			}
+		}
+	}
+	return v
+}
+
+func (v scopeDisjointVisitor) Exit(n INode) {}
+
+func TestParseScopeDisjoint(t *testing.T) {
+	for _, count := range []int{1, 15, 16, 32} {
+		var reads, names strings.Builder
+		for i := 0; i < count; i++ {
+			fmt.Fprintf(&reads, "a%d;", i)
+			if i != 0 {
+				names.WriteString(",")
+			}
+			fmt.Fprintf(&names, "a%d", i)
+		}
+		body := reads.String() + "var " + names.String() + ";outside;"
+		for _, src := range []string{
+			body,
+			"function f(){" + body + "}",
+			"function f(){function g(){" + body + "} " + body + "}",
+			"function f(x=a0){" + body + "}",
+			"(x=a0)=>{" + body + "}",
+			"{" + reads.String() + "let " + names.String() + ";}",
+			"for(let first=a0,a0;outside;){" + reads.String() + "let " + names.String() + ";}",
+			"for(let first=(" + names.String() + ")," + names.String() + ";outside;){}",
+			"class C { m(){" + body + "} }",
+			"switch(outside){case 0:" + reads.String() + "let " + names.String() + ";}",
+		} {
+			for _, inline := range []bool{false, true} {
+				t.Run(fmt.Sprintf("%d/%s/inline=%v", count, src, inline), func(t *testing.T) {
+					ast, err := Parse(parse.NewInputString(src), Options{Inline: inline})
+					if err != nil {
+						t.Fatal(err)
+					}
+					Walk(scopeDisjointVisitor{t}, ast)
+				})
+			}
+		}
+	}
+}
+
 func TestScope(t *testing.T) {
 	js := "let a,b; b = 5; var c; {d}{{d}}"
 	ast, err := Parse(parse.NewInputString(js), Options{})
@@ -1130,12 +1214,12 @@ func TestVariableLookup(t *testing.T) {
 	t.Run("slow Declare", func(t *testing.T) {
 		var sb strings.Builder
 		sb.WriteString("v0")
-		for i := 1; i < 11000; i++ {
-			fmt.Fprintf(&sb, ";v%d", i) // approximately 64 KiB
+		for i := 1; i < 32000; i++ {
+			fmt.Fprintf(&sb, ";v%d", i)
 		}
 		sb.WriteString(";var v0")
-		for i := 1; i < 11000; i++ {
-			fmt.Fprintf(&sb, ",v%d", i) // approximately 64 KiB
+		for i := 1; i < 32000; i++ {
+			fmt.Fprintf(&sb, ",v%d", i)
 		}
 		src := sb.String()
 
@@ -1144,6 +1228,7 @@ func TestVariableLookup(t *testing.T) {
 		test.Error(t, err)
 
 		dur := time.Since(t0)
+		t.Logf("read-then-declare took %v", dur)
 		test.That(t, dur < 1*time.Second, fmt.Sprintf("variable lookup took %v", dur))
 	})
 }