fix(hir): stash class captures after a super() that is not its own statement

changelog.d/8924-capture-stash-after-super.md

@@ -0,0 +1,9 @@
+Place a derived class's early capture stash (`this.__perry_cap_* = param`)
+after the statement that completes `super()` even when the call is not its own
+statement — a minifier's `super(a), this.x = b, …` comma sequence (split so
+the stash sits right after the call), an `if (super(), …)` test, or a `try`.
+The stash used to fall back to constructor entry, which #8643's derived-`this`
+TDZ turned into `ReferenceError: Must call super constructor in derived class
+before accessing 'this' or returning from derived constructor` at every
+construction — Coop's Next.js App Route fixture died at module init on
+`new AppRouteRouteModule({…})` (refs #8546, #8882).

crates/perry-hir/src/lower/tests.rs

@@ -1655,3 +1655,90 @@ fn unresolved_new_names_the_identifier_and_defers_to_a_runtime_global_lookup() {
         "an unresolved constructor must be a runtime globalThis lookup carrying its name:\n{debug}"
     );
 }
+
+/// A derived class with captured outers whose `super()` is not its own
+/// statement — the minifier's `super(a), this.x = b, …` comma sequence, as in
+/// Next's `AppRouteRouteModule` — must stash the `this.__perry_cap_*` fields
+/// AFTER the call, not at constructor entry. #8630's derived-`this` TDZ turns
+/// an entry stash into `ReferenceError: Must call super constructor …` at
+/// every construction (the Coop Next.js fixture died at module init).
+#[test]
+fn derived_ctor_capture_stash_follows_super_inside_comma_sequence() {
+    let source = r#"
+        const exported = (() => {
+            const shared = { tag: "outer" };
+            class Base {
+                constructor(opts) { this.definition = opts.definition; }
+            }
+            class Derived extends Base {
+                constructor({ definition: r, name: n }) {
+                    super({ definition: r }), this.name = n, this.tag = shared.tag;
+                }
+            }
+            return Derived;
+        })();
+    "#;
+    assert_capture_stash_follows_super(source, "Derived");
+}
+
+/// Same requirement for a `super()` nested deeper than a leading comma operand
+/// — p-queue's `if (super(), this.a = 0, …)` shape.
+#[test]
+fn derived_ctor_capture_stash_follows_super_inside_if_test() {
+    let source = r#"
+        const exported = (() => {
+            const shared = { tag: "outer" };
+            class Base {
+                constructor() { this.base = 1; }
+            }
+            class Derived extends Base {
+                constructor(e) {
+                    var q;
+                    if (super(), this.count = 0, this.tag = shared.tag, !e) { q = 1; }
+                    this.q = q;
+                }
+            }
+            return Derived;
+        })();
+    "#;
+    assert_capture_stash_follows_super(source, "Derived");
+}
+
+fn assert_capture_stash_follows_super(source: &str, class_name: &str) {
+    let module = perry_parser::parse_typescript(source, "t.ts").expect("source parses");
+    let hir = super::lower_module(&module, "t", "t.ts").expect("source lowers");
+    let class = hir
+        .classes
+        .iter()
+        .find(|c| c.name == class_name)
+        .unwrap_or_else(|| panic!("fixture declares class {class_name}"));
+    let ctor = class
+        .constructor
+        .as_ref()
+        .expect("the derived class keeps its user-written constructor");
+    let mut super_at = None;
+    let mut first_stash_at = None;
+    for (index, stmt) in ctor.body.iter().enumerate() {
+        let compact: String = format!("{stmt:?}")
+            .chars()
+            .filter(|ch| !ch.is_whitespace())
+            .collect();
+        if super_at.is_none() && compact.contains("SuperCall(") {
+            super_at = Some(index);
+        }
+        if first_stash_at.is_none()
+            && compact.contains("PropertySet{object:This,property:\"__perry_cap_")
+        {
+            first_stash_at = Some(index);
+        }
+    }
+    // Anti-vacuity: the fixture must actually capture (`shared`) and call
+    // `super()`, or the ordering below is not being tested.
+    let super_at = super_at.expect("fixture constructor calls super()");
+    let first_stash_at = first_stash_at.expect("fixture class captures an outer local");
+    assert!(
+        first_stash_at > super_at,
+        "capture stash (stmt {first_stash_at}) must follow super() (stmt {super_at}): {:#?}",
+        ctor.body
+    );
+}

crates/perry-hir/src/lower_decl/class_captures.rs

@@ -552,13 +552,30 @@ pub fn synthesize_class_captures(
     // (#5437). So stash EARLY for intra-ctor method calls AND re-stash at
     // the end / before returns so post-`super()` mutations still win in the
     // final state. The assignments are idempotent.
-    let super_pos = ctor
-        .body
-        .iter()
-        .position(|s| matches!(s, Stmt::Expr(Expr::SuperCall(_) | Expr::SuperCallSpread(_))));
-    let early_insert_at = super_pos.map(|p| p + 1).unwrap_or(rebind_count);
-    for (i, stmt) in assignment_stmts.iter().cloned().enumerate() {
-        ctor.body.insert(early_insert_at + i, stmt);
+    //
+    // In a DERIVED ctor the early stash must sit past the statement that
+    // completes `super()`, whatever shape that call takes. #8630's derived
+    // `this` TDZ (`check_derived_this_initialized`) throws on every `this`
+    // access before `super()` returns, so a stash placed ahead of the call is
+    // no longer a silent write onto the pre-allocated receiver but a
+    // `ReferenceError: Must call super constructor …` at every construction.
+    // Only a call written as its own statement used to be found; minified
+    // bundles fold it into a comma sequence (`super(a), this.x = b, …` —
+    // Next's `AppRouteRouteModule`), an `if (super(), …)` test or a `try`,
+    // all of which landed the stash at constructor entry (#8546 follow-up).
+    let early_insert_at = if has_heritage {
+        // No direct `super()` anywhere in the body (a closure calls it, or a
+        // value-bearing `return` takes the override path): there is no point
+        // at which `this` is known to be bound, so skip the early stash. The
+        // end-of-body and before-`return` stashes below still run.
+        early_capture_stash_slot(&mut ctor.body)
+    } else {
+        Some(rebind_count)
+    };
+    if let Some(early_insert_at) = early_insert_at {
+        for (i, stmt) in assignment_stmts.iter().cloned().enumerate() {
+            ctor.body.insert(early_insert_at + i, stmt);
+        }
     }
     insert_stashes_before_returns(&mut ctor.body, &assignment_stmts);
     for stmt in assignment_stmts {
@@ -589,6 +606,136 @@ pub fn synthesize_class_captures(
     ctx.register_class_captures(name.to_string(), captures_vec);
 }
 
+/// Where the early `this.__perry_cap_* = param` stashes go in a DERIVED
+/// constructor: the index just past the statement that completes `super()`.
+///
+/// Three shapes, in order of preference:
+///
+/// 1. `super(…);` as its own statement — right after it.
+/// 2. A statement whose expression is a comma sequence that STARTS with
+///    `super(…)` (`super(a), this.x = b, …`, the minifier's form): the
+///    sequence is split so the call becomes its own statement, then as (1).
+///    Sound because a statement discards the sequence's value and the
+///    remaining operands still evaluate in order, after the call.
+/// 3. Any other statement that contains a direct `super(…)` (an `if` test, a
+///    `try` body, `_this = super()`): right after that whole statement. `this`
+///    is bound by then; the end-of-body stash still captures later mutations.
+///
+/// `None` when the body has no direct `super()` call.
+fn early_capture_stash_slot(body: &mut Vec<Stmt>) -> Option<usize> {
+    if let Some(p) = body
+        .iter()
+        .position(|s| matches!(s, Stmt::Expr(e) if is_super_call(e)))
+    {
+        return Some(p + 1);
+    }
+    let leading_super_seq = body.iter().position(|s| {
+        matches!(s, Stmt::Expr(Expr::Sequence(items)) if items.first().is_some_and(is_super_call))
+    });
+    if let Some(p) = leading_super_seq {
+        let Stmt::Expr(Expr::Sequence(mut items)) = body.remove(p) else {
+            unreachable!("position matched a leading-super sequence statement");
+        };
+        let call = items.remove(0);
+        body.insert(p, Stmt::Expr(call));
+        match items.len() {
+            0 => {}
+            1 => body.insert(p + 1, Stmt::Expr(items.pop().expect("one operand"))),
+            _ => body.insert(p + 1, Stmt::Expr(Expr::Sequence(items))),
+        }
+        return Some(p + 1);
+    }
+    body.iter()
+        .position(stmt_has_direct_super_call)
+        .map(|p| p + 1)
+}
+
+fn is_super_call(expr: &Expr) -> bool {
+    matches!(expr, Expr::SuperCall(_) | Expr::SuperCallSpread(_))
+}
+
+/// True when `expr` is or contains a `super(…)` call outside nested closures
+/// (`walk_expr_children` does not descend into `Expr::Closure` bodies, which
+/// is the right scope: a closure's `super()` runs when the closure does).
+fn expr_has_direct_super_call(expr: &Expr) -> bool {
+    if is_super_call(expr) {
+        return true;
+    }
+    let mut found = false;
+    crate::walker::walk_expr_children(expr, &mut |child| {
+        if !found && expr_has_direct_super_call(child) {
+            found = true;
+        }
+    });
+    found
+}
+
+fn stmts_have_direct_super_call(stmts: &[Stmt]) -> bool {
+    stmts.iter().any(stmt_has_direct_super_call)
+}
+
+fn stmt_has_direct_super_call(stmt: &Stmt) -> bool {
+    match stmt {
+        Stmt::Expr(e) | Stmt::Throw(e) => expr_has_direct_super_call(e),
+        Stmt::Let { init, .. } => init.as_ref().is_some_and(expr_has_direct_super_call),
+        Stmt::Return(e) => e.as_ref().is_some_and(expr_has_direct_super_call),
+        Stmt::If {
+            condition,
+            then_branch,
+            else_branch,
+        } => {
+            expr_has_direct_super_call(condition)
+                || stmts_have_direct_super_call(then_branch)
+                || else_branch
+                    .as_deref()
+                    .is_some_and(stmts_have_direct_super_call)
+        }
+        Stmt::While { condition, body } | Stmt::DoWhile { body, condition } => {
+            expr_has_direct_super_call(condition) || stmts_have_direct_super_call(body)
+        }
+        Stmt::For {
+            init,
+            condition,
+            update,
+            body,
+        } => {
+            init.as_deref().is_some_and(stmt_has_direct_super_call)
+                || condition.as_ref().is_some_and(expr_has_direct_super_call)
+                || update.as_ref().is_some_and(expr_has_direct_super_call)
+                || stmts_have_direct_super_call(body)
+        }
+        Stmt::Labeled { body, .. } => stmt_has_direct_super_call(body),
+        Stmt::Try {
+            body,
+            catch,
+            finally,
+        } => {
+            stmts_have_direct_super_call(body)
+                || catch
+                    .as_ref()
+                    .is_some_and(|c| stmts_have_direct_super_call(&c.body))
+                || finally.as_deref().is_some_and(stmts_have_direct_super_call)
+        }
+        Stmt::Switch {
+            discriminant,
+            cases,
+        } => {
+            expr_has_direct_super_call(discriminant)
+                || cases.iter().any(|case| {
+                    case.test.as_ref().is_some_and(expr_has_direct_super_call)
+                        || stmts_have_direct_super_call(&case.body)
+                })
+        }
+        Stmt::Break
+        | Stmt::Continue
+        | Stmt::LabeledBreak(_)
+        | Stmt::LabeledContinue(_)
+        | Stmt::PreallocateBoxes(_)
+        | Stmt::PreallocateTdzBoxes(_)
+        | Stmt::ReleaseBoxes(_) => false,
+    }
+}
+
 /// Recursively insert `stashes` immediately before every `Stmt::Return` in
 /// `body` so that cap-field stash assignments run on EVERY early-exit path,
 /// not just the fall-through.  Does not descend into nested function