Fix #751, #753, #754: typed-array index IL-verify, string rest element, assignment destructuring

Compilation/CallHandlers/ArrayDestructureHandler.cs

@@ -8,13 +8,14 @@ namespace SharpTS.Compilation.CallHandlers;
 
 /// <summary>
 /// Handles the internal <c>__arrayDestructure</c> helper that normalizes an array binding-pattern
-/// source through the iterator protocol (#685). Index-addressable sources (arrays, tuples, strings)
-/// are emitted as-is — the desugared positional index access reads them directly and the type checker
+/// source through the iterator protocol (#685). Index-addressable sources (arrays, tuples) are
+/// emitted as-is — the desugared positional index access reads them directly and the type checker
 /// assigns them a matching pass-through type, so no runtime work is needed (preserves the fast path
-/// and tuple positional element types). Every other source is routed through the emitted
-/// <c>ArrayDestructureSource</c> runtime helper, which materializes non-indexable iterables
-/// (generators, Set, Map, <c>[Symbol.iterator]</c> objects) into an array and passes everything else
-/// through unchanged.
+/// and tuple positional element types). Every other source — including <b>strings</b> — is routed
+/// through the emitted <c>ArrayDestructureSource</c> runtime helper, which materializes non-indexable
+/// iterables (generators, Set, Map, strings, <c>[Symbol.iterator]</c> objects) into an array. Strings
+/// are deliberately not on the fast path so a rest element binds a fresh character array rather than
+/// the trailing substring (#753); non-rest character values are identical either way.
 /// </summary>
 public class ArrayDestructureHandler : ICallHandler
 {
@@ -52,6 +53,8 @@ public bool TryHandle(IEmitterContext emitter, Expr.Call call)
         return true;
     }
 
+    // Strings are intentionally excluded: they must materialize through ArrayDestructureSource so a
+    // rest element collects a fresh character array, not the trailing substring (#753).
     private static bool IsIndexAddressable(TypeInfo? type) =>
-        type is TypeInfo.Array or TypeInfo.Tuple or TypeInfo.String or TypeInfo.StringLiteral;
+        type is TypeInfo.Array or TypeInfo.Tuple;
 }

Compilation/ExpressionEmitterBase.cs

@@ -170,6 +170,9 @@ public virtual void EmitExpression(Expr expr)
             case Expr.Comma c:
                 EmitComma(c);
                 break;
+            case Expr.DestructuringAssign da:
+                EmitDestructuringAssign(da);
+                break;
             case Expr.Literal lit:
                 EmitLiteral(lit);
                 break;
@@ -1992,6 +1995,16 @@ protected virtual void EmitComma(Expr.Comma c)
         EmitExpression(c.Right);
         EnsureBoxed();
     }
+
+    /// <summary>
+    /// Emits an assignment-destructuring expression (#754) by running its lowered statements and then
+    /// leaving the result value on the stack. Overridden in <see cref="StatementEmitterBase"/> (the
+    /// concrete emitters' base), which has access to <c>EmitStatement</c>; declared here only so the
+    /// shared <see cref="EmitExpression"/> dispatch can reach it.
+    /// </summary>
+    protected virtual void EmitDestructuringAssign(Expr.DestructuringAssign da) =>
+        throw new NotSupportedException(
+            "DestructuringAssign requires statement emission; emit it from a StatementEmitterBase subclass.");
     #endregion
 
     #region Virtual Methods - Pass-through expressions

Compilation/ILEmitter.Properties.cs

@@ -746,7 +746,14 @@ protected override void EmitGetIndex(Expr.GetIndex gi)
                     EmitExpressionAsDouble(gi.Index);
                     IL.Emit(OpCodes.Conv_I4);
                     IL.Emit(OpCodes.Callvirt, _ctx.Types.GetMethod(listType, "get_Item", _ctx.Types.Int32));
-                    SetStackType(h.Descriptor.StackType);
+                    // Box the unboxed element so this branch converges on `object` with the
+                    // $Array / List<object?> / fallback paths at endLabel. The typed List<T>
+                    // fast path otherwise leaves a native double/bool where the merge point — and
+                    // every consumer, which reads the clobbered StackType=Unknown — expects an
+                    // object ref. That ran only because the typed branch is dead for $Array-backed
+                    // values, but is unverifiable IL (#751).
+                    h.Descriptor.EmitBoxElement(IL, _ctx.Types);
+                    SetStackUnknown();
                     IL.Emit(OpCodes.Br, endLabel);
 
                     // Fallback: type didn't match at loop entry
@@ -787,7 +794,10 @@ protected override void EmitGetIndex(Expr.GetIndex gi)
                 EmitExpressionAsDouble(gi.Index);
                 IL.Emit(OpCodes.Conv_I4);
                 IL.Emit(OpCodes.Callvirt, _ctx.Types.GetMethod(listType, "get_Item", _ctx.Types.Int32));
-                SetStackType(desc.StackType);
+                // Box so this branch converges on `object` with the sibling paths at endLabelNH
+                // (see the hoisted get path above for the full rationale, #751).
+                desc.EmitBoxElement(IL, _ctx.Types);
+                SetStackUnknown();
                 IL.Emit(OpCodes.Br, endLabelNH);
 
                 IL.MarkLabel(notTypedLabel);
@@ -898,7 +908,10 @@ protected override void EmitSetIndex(Expr.SetIndex si)
                 IL.Emit(OpCodes.Ldloc, typedValueLocal);
                 IL.Emit(OpCodes.Call, h.Descriptor.GetSetArrayElementMethod(_ctx.Runtime!));
                 IL.Emit(OpCodes.Ldloc, typedValueLocal);
-                SetStackType(h.Descriptor.StackType);
+                // Box the assigned value so this branch leaves `object` like the fallback path at
+                // endLabel (the assignment result is consumed via StackType=Unknown), #751.
+                h.Descriptor.EmitBoxElement(IL, _ctx.Types);
+                SetStackUnknown();
                 IL.Emit(OpCodes.Br, endLabel);
 
                 // Fallback: type didn't match at loop entry
@@ -970,7 +983,10 @@ protected override void EmitSetIndex(Expr.SetIndex si)
                 IL.Emit(OpCodes.Ldloc, typedValueLocalNH);
                 IL.Emit(OpCodes.Call, desc.GetSetArrayElementMethod(_ctx.Runtime!));
                 IL.Emit(OpCodes.Ldloc, typedValueLocalNH);
-                SetStackType(desc.StackType);
+                // Box so this branch converges on `object` with the sibling paths at endLabelNH
+                // (see the hoisted set path above for the full rationale, #751).
+                desc.EmitBoxElement(IL, _ctx.Types);
+                SetStackUnknown();
                 IL.Emit(OpCodes.Br, endLabelNH);
 
                 // Not typed list: box value and fall through to List<object?> path

Compilation/RuntimeEmitter.Objects.Iteration.cs

@@ -9,13 +9,17 @@ public partial class RuntimeEmitter
 {
     /// <summary>
     /// Emits ArrayDestructureSource: normalizes an array binding-pattern source through the
-    /// iterator protocol (#685). Index-addressable sources — strings and any
+    /// iterator protocol (#685). Index-addressable sources — any
     /// <see cref="System.Collections.IList"/> (arrays, <c>$Array</c>, typed lists) — pass through
     /// unchanged so the desugared positional index access reads them directly and stays consistent
     /// with the matching pass-through type the type checker assigned. Any other iterable (Set, Map,
-    /// generators, <c>[Symbol.iterator]</c> objects, <c>IEnumerable&lt;object&gt;</c>) is materialized
-    /// via <c>IterateToList</c> into a <c>List&lt;object&gt;</c> so positional access yields the
-    /// iterated elements. Non-iterable sources pass through, preserving the existing lenient behavior.
+    /// generators, strings, <c>[Symbol.iterator]</c> objects, <c>IEnumerable&lt;object&gt;</c>) is
+    /// materialized via <c>IterateToList</c> into a <c>List&lt;object&gt;</c> so positional access
+    /// yields the iterated elements. A <b>string</b> is deliberately not on the pass-through path: it
+    /// materializes to a fresh character array so a rest element binds an array rather than the trailing
+    /// substring (<c>const [a, ...rest] = "hi"</c>), matching ECMA-262 (#753) — non-rest character
+    /// values are identical either way. Non-iterable sources pass through, preserving the existing
+    /// lenient behavior.
     /// Signature: object ArrayDestructureSource(object value, $TSSymbol iteratorSymbol, Type runtimeType)
     /// </summary>
     private void EmitArrayDestructureSource(TypeBuilder typeBuilder, EmittedRuntime runtime)
@@ -33,13 +37,10 @@ private void EmitArrayDestructureSource(TypeBuilder typeBuilder, EmittedRuntime
 
         var passThroughLabel = il.DefineLabel();
 
-        // string → pass through (the source stays typed as string; index access reads chars).
-        il.Emit(OpCodes.Ldarg_0);
-        il.Emit(OpCodes.Isinst, _types.String);
-        il.Emit(OpCodes.Brtrue, passThroughLabel);
-
         // IList (List<object>, $Array, typed lists) → pass through: already index-addressable, and
         // routing a typed list (List<double>/List<bool>) through IterateToList would re-box it.
+        // Note: a .NET string is NOT an IList, so it falls through to the IterateToList path below and
+        // is materialized into a character array (#753) — required so a rest element binds an array.
         il.Emit(OpCodes.Ldarg_0);
         il.Emit(OpCodes.Isinst, ilistType);
         il.Emit(OpCodes.Brtrue, passThroughLabel);

Compilation/RuntimeFeatureDetector.cs

@@ -824,6 +824,13 @@ private void VisitExpr(Expr? expr)
                 VisitExpr(cm.Left);
                 VisitExpr(cm.Right);
                 break;
+            case Expr.DestructuringAssign da:
+                // Walk the lowered assignment statements so a feature inside the rhs/targets
+                // (e.g. eval/Proxy in the source expression) is still detected (#754).
+                foreach (var s in da.Assignments)
+                    VisitStmt(s);
+                VisitExpr(da.ResultValue);
+                break;
             case Expr.Grouping gr:
                 VisitExpr(gr.Expression);
                 break;

Compilation/StatementEmitterBase.cs

@@ -190,6 +190,20 @@ protected virtual void EmitTruthyCheck()
 
     #region Core Statement Dispatch
 
+    /// <summary>
+    /// Emits an assignment-destructuring expression (#754): run the lowered assignment statements (temp
+    /// binding + per-target writes, all synthesized <see cref="Stmt.Var"/>/<see cref="Stmt.Expression"/>
+    /// with no control flow), then leave the result value (the original rhs) on the stack. Each statement
+    /// goes through the normal <see cref="EmitStatement"/> path, so the async/generator subclasses handle
+    /// an <c>await</c>/<c>yield</c> in the rhs without any extra plumbing.
+    /// </summary>
+    protected override void EmitDestructuringAssign(Expr.DestructuringAssign da)
+    {
+        foreach (var stmt in da.Assignments)
+            EmitStatement(stmt);
+        EmitExpression(da.ResultValue);
+    }
+
     /// <summary>
     /// Dispatches statement emission to the appropriate handler method.
     /// </summary>

Execution/Interpreter.Expressions.cs

@@ -47,6 +47,14 @@ internal RuntimeValue EvaluateRV(Expr expr)
     // All Evaluate* methods return RuntimeValue directly — no FromBoxed in dispatch.
 
     internal RuntimeValue VisitComma(Expr.Comma comma) { Evaluate(comma.Left); return EvaluateRV(comma.Right); }
+    internal RuntimeValue VisitDestructuringAssign(Expr.DestructuringAssign d)
+    {
+        // Run the lowered assignment statements (temp binding + per-target writes), then yield the
+        // original rhs the temp holds — an assignment expression evaluates to its right-hand side (#754).
+        foreach (var stmt in d.Assignments)
+            Execute(stmt);
+        return EvaluateRV(d.ResultValue);
+    }
     internal RuntimeValue VisitBinary(Expr.Binary binary) => EvaluateBinary(binary);
     internal RuntimeValue VisitLogical(Expr.Logical logical) => EvaluateLogical(logical);
     internal RuntimeValue VisitNullishCoalescing(Expr.NullishCoalescing nc) => EvaluateNullishCoalescing(nc);
@@ -115,6 +123,12 @@ internal async Task<RuntimeValue> EvaluateAsync(Expr expr)
         switch (expr)
         {
             case Expr.Comma comma: await EvaluateAsync(comma.Left); return await EvaluateAsync(comma.Right);
+            case Expr.DestructuringAssign d:
+                // Lowered statements may contain `await` in the rhs; run them on the async path, then
+                // yield the temp holding the original rhs (#754).
+                foreach (var stmt in d.Assignments)
+                    await ExecuteStatementAsync(stmt);
+                return await EvaluateAsync(d.ResultValue);
             case Expr.Binary binary: return await EvaluateBinaryAsync(binary);
             case Expr.Logical logical: return await EvaluateLogicalAsync(logical);
             case Expr.NullishCoalescing nc: return await EvaluateNullishCoalescingAsync(nc);

Execution/Interpreter.Statements.cs

@@ -945,15 +945,19 @@ private ExecutionResult ExecuteForIn(Stmt.ForIn forIn)
     /// <summary>
     /// Normalizes an array binding-pattern source through the iterator protocol (#685). Array
     /// destructuring (<c>const [a, b] = src</c>) desugars to positional index access, which is only
-    /// correct for index-addressable sources. Arrays, strings, typed arrays and buffers pass through
-    /// unchanged (fast path); any other source is materialized into a <see cref="SharpTSArray"/> via
-    /// <see cref="GetIterableElements"/> — the same routine spread uses — so the index access reads
-    /// the iterated elements. A genuinely non-iterable source throws "is not iterable", matching JS;
-    /// the type checker already rejects those statically except behind <c>any</c>.
+    /// correct for index-addressable sources. Arrays, typed arrays and buffers pass through unchanged
+    /// (fast path); any other source — including <b>strings</b> — is materialized into a
+    /// <see cref="SharpTSArray"/> via <see cref="GetIterableElements"/> (the same routine spread uses)
+    /// so the index access reads the iterated elements. Strings are intentionally materialized (not on
+    /// the fast path) so a rest element binds a fresh array of characters rather than the trailing
+    /// substring (<c>const [a, ...rest] = "hi"</c> → <c>rest = ["i"]</c>), matching ECMA-262 (#753);
+    /// non-rest character values are identical either way. A genuinely non-iterable source throws "is
+    /// not iterable", matching JS; the type checker already rejects those statically except behind
+    /// <c>any</c>.
     /// </summary>
     internal object? NormalizeArrayDestructureSource(object? value)
     {
-        if (value is SharpTSArray or string or SharpTSTypedArray or SharpTSBuffer)
+        if (value is SharpTSArray or SharpTSTypedArray or SharpTSBuffer)
             return value;
 
         return new SharpTSArray(GetIterableElements(value).ToList());

Parsing/AST.cs

@@ -49,6 +49,7 @@ public abstract record Expr
     public static TResult Accept<TResult>(Expr expr, IExprVisitor<TResult> visitor) => expr switch
     {
         Comma e => visitor.VisitComma(e),
+        DestructuringAssign e => visitor.VisitDestructuringAssign(e),
         Binary e => visitor.VisitBinary(e),
         Logical e => visitor.VisitLogical(e),
         NullishCoalescing e => visitor.VisitNullishCoalescing(e),
@@ -98,6 +99,20 @@ public abstract record Expr
 
     /// <summary>Comma (sequence) expression: evaluates all sub-expressions left-to-right, returns the last value.</summary>
     public record Comma(Expr Left, Expr Right) : Expr;
+    /// <summary>
+    /// Destructuring assignment to existing l-values — <c>[a, b] = rhs</c> / <c>({a, b} = rhs)</c> (#754).
+    /// Unlike declaration destructuring (which binds new variables), the targets are existing
+    /// <see cref="Variable"/>/<see cref="Get"/>/<see cref="GetIndex"/> l-values. The parser lowers the
+    /// pattern (already parsed as an array/object literal) into <paramref name="Assignments"/> — a temp
+    /// declaration for the rhs, the per-target assignment statements (reusing the #685 iterator-protocol
+    /// normalization <c>__arrayDestructure</c> and <c>__objectRest</c>), and any nested temps — plus
+    /// <paramref name="ResultValue"/>, the temp holding the original rhs (an assignment expression
+    /// evaluates to its right-hand side, per ECMA-262). Every backend lowers it identically: run
+    /// <c>Assignments</c>, then yield <c>ResultValue</c>. <c>Assignments</c> contains only synthesized
+    /// <see cref="Stmt.Var"/> and <see cref="Stmt.Expression"/> statements (no control flow), so it is
+    /// safe in any expression position.
+    /// </summary>
+    public record DestructuringAssign(List<Stmt> Assignments, Expr ResultValue) : Expr;
     public record Binary(Expr Left, Token Operator, Expr Right) : Expr;
     public record Logical(Expr Left, Token Operator, Expr Right) : Expr;
     public record NullishCoalescing(Expr Left, Expr Right) : Expr;