diff --git a/Compilation/ArrayLocalPromotionAnalyzer.cs b/Compilation/ArrayLocalPromotionAnalyzer.cs index 8a7efb30..65aca306 100644 --- a/Compilation/ArrayLocalPromotionAnalyzer.cs +++ b/Compilation/ArrayLocalPromotionAnalyzer.cs @@ -22,8 +22,9 @@ namespace SharpTS.Compilation; /// x.push(...) — any other appearance of the bare variable (argument pass, return, store, /// spread, for…of, ===, reassignment, delete x[i], pop/any other /// method or property) disqualifies it; -/// the name is declared exactly once in the whole program (conservative guard against scope -/// ambiguity / shadowing without full scope resolution); +/// the name is declared exactly once within its function scope (conservative guard against +/// shadowing without full scope resolution; candidacy is keyed per scope, so a same-named +/// array in a different function/module never interferes); /// the name is not captured by any closure (a captured local is routed to an object /// display-class field, never a typed slot). /// @@ -42,12 +43,12 @@ public static void Analyze(List program, TypeMap? typeMap, ClosureAnalyzer foreach (var stmt in program) visitor.Visit(stmt); - foreach (var (name, nameToken) in visitor.Candidates) + foreach (var (key, nameToken) in visitor.Candidates) { - if (visitor.Disqualified.Contains(name)) continue; - if (visitor.DeclCount.GetValueOrDefault(name) != 1) continue; - if (!visitor.ElementToken.TryGetValue(name, out var token)) continue; // no Double/Bool use seen - if (closures?.IsVariableCaptured(name) == true) continue; + if (visitor.Disqualified.Contains(key)) continue; + if (visitor.DeclCount.GetValueOrDefault(key) != 1) continue; + if (!visitor.ElementToken.TryGetValue(key, out var token)) continue; // no Double/Bool use seen + if (closures?.IsVariableCaptured(key.Name) == true) continue; typeMap.MarkPromotableArrayLocal(nameToken, token); } } @@ -56,17 +57,36 @@ private sealed class Visitor(TypeMap typeMap) : AstVisitorBase { private readonly TypeMap _typeMap = typeMap; - /// name → candidate declaration's name token (empty-array-literal local). - public Dictionary Candidates { get; } = new(); + // Candidacy/disqualification is keyed by (function scope, lexeme), NOT whole-program lexeme: a + // common array name like `arr`/`data` in one function must not be poisoned by an unrelated, + // escaping same-name array in another (e.g. across bundled modules). Each function/arrow body is + // its own scope; cross-scope references are captures, handled by the IsVariableCaptured guard. The + // module top level is scope 0. Mirrors StringAccumulatorPromotionAnalyzer. + private int _scope; + private int _nextScope; - /// How many times each name is declared anywhere (any kind of binding). - public Dictionary DeclCount { get; } = new(); + /// (scope, name) → candidate declaration's name token (empty-array-literal local). + public Dictionary<(int Scope, string Name), Token> Candidates { get; } = new(); - /// name → element primitive token (TYPE_NUMBER / TYPE_BOOLEAN), from its first typed use. - public Dictionary ElementToken { get; } = new(); + /// How many times each (scope, name) is declared. + public Dictionary<(int Scope, string Name), int> DeclCount { get; } = new(); - /// Names with at least one disqualifying occurrence. - public HashSet Disqualified { get; } = new(); + /// (scope, name) → element primitive token (TYPE_NUMBER / TYPE_BOOLEAN), from its first typed use. + public Dictionary<(int Scope, string Name), TokenType> ElementToken { get; } = new(); + + /// (scope, name) pairs with at least one disqualifying occurrence. + public HashSet<(int Scope, string Name)> Disqualified { get; } = new(); + + protected override void VisitFunction(Stmt.Function stmt) => InScope(() => base.VisitFunction(stmt)); + protected override void VisitArrowFunction(Expr.ArrowFunction expr) => InScope(() => base.VisitArrowFunction(expr)); + + private void InScope(Action body) + { + var saved = _scope; + _scope = ++_nextScope; + body(); + _scope = saved; + } protected override void VisitVar(Stmt.Var stmt) => HandleDeclaration(stmt.Name, stmt.Initializer); @@ -76,11 +96,11 @@ protected override void VisitConst(Stmt.Const stmt) => private void HandleDeclaration(Token name, Expr? initializer) { - var lexeme = name.Lexeme; - DeclCount[lexeme] = DeclCount.GetValueOrDefault(lexeme) + 1; + var key = (_scope, name.Lexeme); + DeclCount[key] = DeclCount.GetValueOrDefault(key) + 1; - if (initializer is Expr.ArrayLiteral { Elements.Count: 0 } && !Candidates.ContainsKey(lexeme)) - Candidates[lexeme] = name; + if (initializer is Expr.ArrayLiteral { Elements.Count: 0 } && !Candidates.ContainsKey(key)) + Candidates[key] = name; // Visit the initializer for completeness ([] has no sub-uses, but a // non-candidate initializer may reference other arrays). @@ -110,7 +130,7 @@ protected override void VisitSetIndex(Expr.SetIndex expr) // diverging from the general path that preserves it. Disqualify — the // array analogue of the #367/#372 numeric-slot taint guard. if (ValueAdmitsUndefinedSentinel(expr.Value)) - Disqualified.Add(v.Name.Lexeme); + Disqualified.Add((_scope, v.Name.Lexeme)); } else Visit(expr.Object); @@ -147,7 +167,7 @@ protected override void VisitCall(Expr.Call expr) foreach (var arg in expr.Arguments) { if (ValueAdmitsUndefinedSentinel(arg)) - Disqualified.Add(v.Name.Lexeme); + Disqualified.Add((_scope, v.Name.Lexeme)); Visit(arg); } return; @@ -160,13 +180,13 @@ protected override void VisitAssign(Expr.Assign expr) // `x = ...` rebinds the slot — disqualify. (Only the empty-literal // initializer is supported; any later store could be a $Array or a // value the typed slot can't hold.) - Disqualified.Add(expr.Name.Lexeme); + Disqualified.Add((_scope, expr.Name.Lexeme)); base.VisitAssign(expr); } protected override void VisitCompoundAssign(Expr.CompoundAssign expr) { - Disqualified.Add(expr.Name.Lexeme); + Disqualified.Add((_scope, expr.Name.Lexeme)); base.VisitCompoundAssign(expr); } @@ -182,7 +202,7 @@ protected override void VisitDelete(Expr.Delete expr) break; } if (target is Expr.Variable v) - Disqualified.Add(v.Name.Lexeme); + Disqualified.Add((_scope, v.Name.Lexeme)); base.VisitDelete(expr); } @@ -190,7 +210,7 @@ protected override void VisitVariable(Expr.Variable expr) { // Catch-all: any bare variable occurrence not consumed by a permitted-use // override above is an escape (returned, passed, spread, compared, etc.). - Disqualified.Add(expr.Name.Lexeme); + Disqualified.Add((_scope, expr.Name.Lexeme)); } private void NotePermittedReceiver(Expr.Variable v) @@ -198,15 +218,15 @@ private void NotePermittedReceiver(Expr.Variable v) // Resolve the element kind from the receiver's static array type (as // ArrayHoistAnalyzer does). Only Double/Bool arrays are promotable; an // Object-kind array (string[]/union[]) disqualifies the name. - if (ElementToken.ContainsKey(v.Name.Lexeme)) return; + if (ElementToken.ContainsKey((_scope, v.Name.Lexeme))) return; var desc = ArrayElements.Resolve(_typeMap.Get(v)); if (desc == null) return; // not statically an array here — leave undecided if (desc.Kind == ArrayElementsKind.Object || desc.ElementTokenType is not { } tok) { - Disqualified.Add(v.Name.Lexeme); + Disqualified.Add((_scope, v.Name.Lexeme)); return; } - ElementToken[v.Name.Lexeme] = tok; + ElementToken[(_scope, v.Name.Lexeme)] = tok; } /// diff --git a/SharpTS.Tests/SharedTests/ArrayLocalPromotionTests.cs b/SharpTS.Tests/SharedTests/ArrayLocalPromotionTests.cs index 622b9f40..dc8ad67f 100644 --- a/SharpTS.Tests/SharedTests/ArrayLocalPromotionTests.cs +++ b/SharpTS.Tests/SharedTests/ArrayLocalPromotionTests.cs @@ -244,4 +244,33 @@ function f(): number { Assert.Equal("6\n", TestHarness.Run(source, mode)); } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void Promoted_PerScope_NameCollisionDoesNotPoison(ExecutionMode mode) + { + // `xs` in build() is a clean push/index/length array and must promote even though an + // unrelated, escaping `xs` (returned) exists in leak() — candidacy is keyed per function + // scope, so a common array name shared across functions/modules no longer poisons the + // whole program. Both must produce correct results. + var source = """ + function leak(): number[] { + const xs: number[] = []; + xs.push(9); + return xs; + } + function build(): number { + const xs: number[] = []; + for (let i: number = 0; i < 5; i++) { xs.push(i); } + let sum: number = 0; + for (let i: number = 0; i < xs.length; i++) { sum = sum + xs[i]; } + return sum; + } + console.log(build()); + console.log(leak()[0]); + """; + + // 0+1+2+3+4 = 10 ; leak()[0] = 9 + Assert.Equal("10\n9\n", TestHarness.Run(source, mode)); + } } diff --git a/docs/plans/issue-861-array-methods-typed-pipeline.md b/docs/plans/issue-861-array-methods-typed-pipeline.md new file mode 100644 index 00000000..f36f74fc --- /dev/null +++ b/docs/plans/issue-861-array-methods-typed-pipeline.md @@ -0,0 +1,71 @@ +# Plan: typed `List` HOF pipeline for monomorphic number arrays (#861 follow-on / #856 child) + +## Why + +After strings (#857), **array-methods is the last benchmark workload meaningfully off Node** (everything else is at parity or we win): + +| array-methods @100k (warm) | Compiled | Node | Slowdown | +|---|---|---|---| +| full pipeline | 11.2 | 4.0 | 2.79× | + +Stage decomposition (mean ms @100k, content-forced) localizes the gap: + +| stage | Compiled | Node | Slowdown | +|---|---|---|---| +| build (`push` loop) | 6.0 | 1.5 | 3.9× | +| `map` | 4.9 | 1.1 | 4.4× | +| `filter` | 9.4 | 1.2 | **7.9×** | +| `reduce` | 1.1 | 1.0 | ~parity ✅ | + +**Root cause (confirmed from IL of `filterOnly`):** the whole pipeline is **boxed `double` in `List`**. `arr` is `$Array`-backed (not promoted to `List` — it's consumed by `.map`, which the `ArrayLocalPromotionAnalyzer` escape rule disqualifies). `filter` calls `ArrayFilterDirectBool(List, Func)`: every element is unboxed inside the adapter and the kept ones re-boxed into a fresh `List`. `map` is analogous. #861 already removed the callback-dispatch overhead (bool-returning adapter, no per-stage `List↔$Array` round-trip) — the **residual gap is purely per-element boxing**, so there is no surgical fix; the representation must become typed. + +`reduce` is already at parity because it folds to a scalar (no result array to box). + +## Goal + +When a number array flows through `map`/`filter`/`reduce` with monomorphic `number`→`number` / `number`→`bool` callbacks, keep it as a concrete `List` end-to-end: no per-element box/unbox, no `$Array` indirection. + +## Design + +Three coordinated pieces (analyzer → emitter → runtime helpers): + +### 1. Promotion analyzer — permit terminal HOF consumers + +Extend the typed-array-local promotion (currently `push`/`[i]`/`.length` only) so a `number[]`/`boolean[]` local may also be the **receiver of `map`/`filter`/`reduce`** and still stay `List`/`List` — provided the array identity itself doesn't escape (the HOF consumes it and returns a *new* array/scalar; the original isn't returned/passed/aliased). `forEach` can join later. + +- Reuse the same conservative escape discipline as `ArrayLocalPromotionAnalyzer` (permitted-use overrides that consume the receiver without recursing into it). +- **Prerequisite:** port the **per-function-scope candidacy** fix from `StringAccumulatorPromotionAnalyzer` to `ArrayLocalPromotionAnalyzer` first (see its own note) — without it, common array names (`arr`, `data`, `items`) are poisoned across bundled modules and never promote anyway. + +### 2. Typed HOF result locals + +`const doubled = arr.map(...)` / `const evens = doubled.filter(...)` must themselves be `List` slots, not `object`. Mark the HOF-result local as promoted when (a) the receiver is a typed `List`/`List` and (b) the callback's return type is statically `number`/`boolean`. This chains: `arr`(List) → `doubled`(List) → `evens`(List) → `reduce`→`double`. + +### 3. Typed runtime HOF helpers + emitter dispatch + +Add typed helpers alongside the existing `ArrayFilterDirectBool` etc. (in `RuntimeEmitter.Arrays.*` / the `$Runtime` HOF surface): +- `ArrayMapDouble(List, Func) -> List` +- `ArrayFilterDouble(List, Func) -> List` +- `ArrayReduceDouble(List, Func, double) -> double` +- (`bool` variants as needed) + +Emitter (`ILEmitter.Calls.MethodDispatch.cs`, the #861 HOF path): when the receiver slot is `List` and the callback is a monomorphic `double`-typed arrow, bind a `Func`/`Func` (the #858/#861 de-virtualized arrow already gives a direct delegate) and call the typed helper. The callback bodies are already numeric (the #861 typed-adapter work) — here the adapter signature becomes `double`-based instead of `object`-based, eliminating the box at the call boundary and the unbox inside. + +Keep the existing `object`/`$Array` helpers as the fallback for polymorphic / non-promoted arrays. + +## Expected impact + +Eliminating per-element boxing across build+map+filter should pull each from 4–8× toward `reduce`'s ~parity, i.e. array-methods @100k from ~11 ms toward ~5 ms (≈Node). Bounded by the inherent intermediate-array allocations (which Node also pays). + +## Correctness & gates + +- `$Array` semantics (holes, sparseness, identity) must never be observed for a promoted `List` — same non-escaping guard as the existing array promotion. OOB and `undefined`-sentinel handling per the #857/#860 landmines (bounds-checked reads; a `double` slot can't carry `$Undefined`). +- Callback return-type monomorphism must be statically proven; a callback returning `number | undefined` etc. disqualifies (falls back to the boxed path). +- Green on `dotnet test`, `SharpTS.Test262` (interp + compiled), `SharpTS.TypeScriptConformance`; `--verify` the emitted HOF chains. +- Measure content-forced, with the stage decomposition above as before/after. + +## Scope / sequencing + +High value (last benchmark gap), **medium-high risk** (touches the HOF hot path + the promotion analyzer). Larger than #857. Suggested order: +1. Port per-scope candidacy to `ArrayLocalPromotionAnalyzer` (small, low-risk, independently shippable; also hardens existing bundled-program promotion). +2. Typed `map`→`reduce` (no `filter`) as the first vertical slice (proves the typed-delegate + helper plumbing on the simplest chain). +3. Add `filter` (the worst stage) and result-local chaining.