From c9fc83bcbeac11e5bf5fe47113ee8101d39a72c5 Mon Sep 17 00:00:00 2001 From: Joshua Rubin Date: Fri, 16 Oct 2026 00:12:44 +0300 Subject: [PATCH] fix(js): clean up undeclared variables at scope exit Defer indexed declaration cleanup until scope close, including global scopes. Preserve outer references and argument-use offsets. Add scope regression checks and strengthen the timing guard. --- js/ast.go | 46 ++++++++++++++---------- js/parse.go | 2 ++ js/parse_test.go | 93 +++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 119 insertions(+), 22 deletions(-) diff --git a/js/ast.go b/js/ast.go index 18bae08..35618a2 100644 --- a/js/ast.go +++ b/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 { diff --git a/js/parse.go b/js/parse.go index ee2dd16..fce43da 100644 --- a/js/parse.go +++ b/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() diff --git a/js/parse_test.go b/js/parse_test.go index e589a32..1ae8e0a 100644 --- a/js/parse_test.go +++ b/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)) }) }