From bebe759c633e757e20b10537c2c80554eabd8960 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 10 Jun 2026 15:10:58 +0200 Subject: [PATCH] fix(hir): non-class statement-semantics test262 remnant Three root-cause fixes across the language/statements tail (function / for-of / do-while / while), +7 tests, zero regressions (verified vs main on all 16 statement dirs and the built-ins+language 0/12 shard). 1. typeof(x) with a parenthesized operand. The typeof AST folds matched a bare Ident/Member, so `typeof(zzz)` fell through to a normal operand lowering and emitted a ReferenceError-throwing get instead of folding an unresolved identifier to "undefined" (spec GetValue-skips-on-typeof). Peel transparent Paren wrappers before the folds. Fixes do-while/S12.6.1_A10, while/S12.6.2_A10. 2. for-of IteratorClose on a throw completion. The lazy iterator-protocol loop ran IteratorClose for break/return/labeled abrupts but not when the body threw (explicit `throw` or a runtime exception). Wrap only the user body in a try/catch that runs an unvalidated, exception-swallowing IteratorClose and re-throws; the element-.value read and binding stay outside, since spec IteratorValue throwing sets the iterator done WITHOUT closing. Applied to both for-of lowering paths (module-init and body/block). Fixes for-of/iterator-close-via-throw, for-of/generator-close-via-throw. 3. f.prototype.constructor read returned undefined. The inline nested read folds to GetFunctionPrototypeMethod, whose runtime entry only served registered methods + a builtin allowlist and returned undefined for "constructor" (and never materialized the prototype, so cid was 0). Add a constructor arm that routes through js_function_prototype_value_for_read (respecting a replaced f.prototype = X) and reads its constructor field. Fixes function/S13.2_A4_T1, function/S13.2_A4_T2, function/13.2-17-1. --- crates/perry-hir/src/lower/lower_expr.rs | 20 ++++- crates/perry-hir/src/lower/mod.rs | 1 + crates/perry-hir/src/lower/stmt_loops.rs | 87 +++++++++++++++++-- crates/perry-hir/src/lower_decl/body_stmt.rs | 18 ++-- .../src/object/class_registry.rs | 24 +++++ 5 files changed, 133 insertions(+), 17 deletions(-) diff --git a/crates/perry-hir/src/lower/lower_expr.rs b/crates/perry-hir/src/lower/lower_expr.rs index 7b8ac57b85..6810cb5e1f 100644 --- a/crates/perry-hir/src/lower/lower_expr.rs +++ b/crates/perry-hir/src/lower/lower_expr.rs @@ -896,11 +896,25 @@ pub(crate) fn lower_expr(ctx: &mut LoweringContext, expr: &ast::Expr) -> Result< // checks fail). The static methods are real functions in // Node, so fold to the literal "function" string here. if matches!(unary.op, ast::UnaryOp::TypeOf) { + // `typeof(x)` parenthesizes the operand, so the AST-level folds + // below — which match a bare `Ident` / `Member` — would miss it + // and fall through to a normal operand lowering. For an + // unresolved identifier that means `typeof(zzz)` emitted a + // ReferenceError-throwing get instead of folding to "undefined" + // (the spec's GetValue-skips-on-typeof rule). Peel transparent + // `Paren` wrappers so the operand-shape folds see through them. + let typeof_arg = { + let mut e = unary.arg.as_ref(); + while let ast::Expr::Paren(p) = e { + e = p.expr.as_ref(); + } + e + }; // #677: bare `typeof Function` — Function is a JS built-in // constructor, so typeof is "function". Without this fold, // the bare ident lowers to `GlobalGet(0)` and typeof reads // "object" via the global-this short-circuit. - if let ast::Expr::Ident(id) = unary.arg.as_ref() { + if let ast::Expr::Ident(id) = typeof_arg { if id.sym.as_ref() == "Function" && ctx.lookup_local("Function").is_none() { return Ok(Expr::String("function".to_string())); } @@ -980,7 +994,7 @@ pub(crate) fn lower_expr(ctx: &mut LoweringContext, expr: &ast::Expr) -> Result< // (`(process.memoryUsage).rss`) so it bypasses the // ident-receiver fold below. Node exposes `rss` as a fast-path // function hung off `process.memoryUsage`; fold to "function". - if let ast::Expr::Member(outer) = unary.arg.as_ref() { + if let ast::Expr::Member(outer) = typeof_arg { if let ast::MemberProp::Ident(outer_prop) = &outer.prop { if outer_prop.sym.as_ref() == "rss" { if let ast::Expr::Member(inner) = outer.obj.as_ref() { @@ -998,7 +1012,7 @@ pub(crate) fn lower_expr(ctx: &mut LoweringContext, expr: &ast::Expr) -> Result< } } } - if let ast::Expr::Member(member) = unary.arg.as_ref() { + if let ast::Expr::Member(member) = typeof_arg { if let ast::Expr::Ident(obj_ident) = member.obj.as_ref() { if let ast::MemberProp::Ident(prop_ident) = &member.prop { let obj_name = obj_ident.sym.as_ref(); diff --git a/crates/perry-hir/src/lower/mod.rs b/crates/perry-hir/src/lower/mod.rs index edd3896a6b..affdbb65b7 100644 --- a/crates/perry-hir/src/lower/mod.rs +++ b/crates/perry-hir/src/lower/mod.rs @@ -54,6 +54,7 @@ mod stmt_loops; pub(crate) use stmt_loops::{ insert_iterator_close_on_abrupt, iterator_close_guarded_stmt, iterator_next_call, lazy_iter_for_stmt, lazy_or_index_elem, lower_stmt_for_in, lower_stmt_for_of, + wrap_lazy_for_of_body_close_on_throw, }; mod module_decl; pub(crate) use module_decl::*; diff --git a/crates/perry-hir/src/lower/stmt_loops.rs b/crates/perry-hir/src/lower/stmt_loops.rs index 3e6e37ed2e..a385492999 100644 --- a/crates/perry-hir/src/lower/stmt_loops.rs +++ b/crates/perry-hir/src/lower/stmt_loops.rs @@ -410,6 +410,70 @@ pub(crate) fn iterator_close_guarded_stmt(iter_id: LocalId) -> Stmt { } } +/// Wrap a lazy `for...of` body (binding + user statements) in a `try/catch` +/// that runs IteratorClose when the body completes abruptly with a *throw* — +/// either an explicit `throw` statement or a runtime exception (a throwing +/// setter in the LHS `PutValue`, a destructuring error, an assertion failure, +/// a generator `.throw()` propagation). The `break`/`return`/labeled cases are +/// handled separately by `insert_iterator_close_on_abrupt` (they are control +/// flow, not exceptions, so this `catch` never sees them — no double-close). +/// +/// Per spec, for a throw completion IteratorClose invokes `return` but does +/// NOT validate its result and SWALLOWS any exception it raises — the original +/// throw is the one that propagates. So the close here is unvalidated and +/// itself wrapped in a result-swallowing `try/catch`, then the caught error is +/// re-thrown. +pub(crate) fn wrap_lazy_for_of_body_close_on_throw( + ctx: &mut LoweringContext, + iter_id: LocalId, + body: Vec, +) -> Stmt { + let err_id = ctx.fresh_local(); + let err_name = format!("__forof_err_{}", err_id); + ctx.locals.push((err_name.clone(), err_id, Type::Any)); + let ret_err_id = ctx.fresh_local(); + let ret_err_name = format!("__forof_ret_err_{}", ret_err_id); + ctx.locals + .push((ret_err_name.clone(), ret_err_id, Type::Any)); + + // try { if (__iter.return != null) __iter.return(); } catch (_) {} + // + // The whole close — including the `__iter.return` *read* (which may be an + // accessor that throws) and the call's result — is inside the swallowing + // `try`: for a throw completion the close's own abrupt completion is + // discarded and the ORIGINAL throw propagates (spec IteratorClose, throw + // case). Keeping the `.return` read outside would let a throwing getter + // (`iterator-close-throw-get-method-abrupt`) replace the original error. + let guarded_close = Stmt::Try { + body: vec![Stmt::If { + condition: Expr::Compare { + op: CompareOp::LooseNe, + left: Box::new(Expr::PropertyGet { + object: Box::new(Expr::LocalGet(iter_id)), + property: "return".to_string(), + }), + right: Box::new(Expr::Null), + }, + then_branch: vec![Stmt::Expr(iterator_return_call(iter_id, false))], + else_branch: None, + }], + catch: Some(CatchClause { + param: Some((ret_err_id, ret_err_name)), + body: Vec::new(), + }), + finally: None, + }; + + Stmt::Try { + body, + catch: Some(CatchClause { + param: Some((err_id, err_name)), + body: vec![guarded_close, Stmt::Throw(Expr::LocalGet(err_id))], + }), + finally: None, + } +} + /// Rewrite a synchronous `for...of` body so every abrupt completion that /// escapes the loop runs IteratorClose first. Per spec ForIn/OfBodyEvaluation: /// an unlabeled `break` that targets this loop, a labeled `break`/`continue` @@ -1703,6 +1767,21 @@ pub(crate) fn lower_stmt_for_of( // escaping the loop runs IteratorClose (`__iter.return()`) first. if use_lazy_iter { insert_iterator_close_on_abrupt(&mut loop_body, arr_id, 0, &[]); + // Wrap ONLY the user body so a throw escaping it runs IteratorClose. + // break/return/labeled abrupts were already handled above; this covers + // the throw-completion case those intentionally leave alone. The + // element-`.value` read and binding statements stay OUTSIDE the wrapper: + // per spec, IteratorValue throwing sets the iterator done and does NOT + // close it (`iterator-next-result-value-attr-error`) — only an abrupt + // body completion does. + let guarded_body = wrap_lazy_for_of_body_close_on_throw(ctx, arr_id, loop_body); + let mut full_body = binding_stmts; + full_body.push(guarded_body); + module + .init + .push(lazy_iter_for_stmt(arr_id, result_id, full_body)); + ctx.pop_block_scope(for_scope_mark); + return Ok(()); } // Prepend the binding statements to the loop body @@ -1710,14 +1789,6 @@ pub(crate) fn lower_stmt_for_of( loop_body.insert(i, stmt); } - if use_lazy_iter { - module - .init - .push(lazy_iter_for_stmt(arr_id, result_id, loop_body)); - ctx.pop_block_scope(for_scope_mark); - return Ok(()); - } - // Loop bound. Map/Set fast paths read `.size` (lowered by // codegen to `js_map_size` / `js_set_size`); regular path uses // `__arr.length` against the materialized iterable. diff --git a/crates/perry-hir/src/lower_decl/body_stmt.rs b/crates/perry-hir/src/lower_decl/body_stmt.rs index 265d7b3fe0..0eb76fa194 100644 --- a/crates/perry-hir/src/lower_decl/body_stmt.rs +++ b/crates/perry-hir/src/lower_decl/body_stmt.rs @@ -7,7 +7,8 @@ use crate::destructuring::*; use crate::ir::*; use crate::lower::{ collect_for_of_pattern_leaves, emit_for_of_pattern_binding, insert_iterator_close_on_abrupt, - lazy_iter_for_stmt, lazy_or_index_elem, lower_expr, LoweringContext, + lazy_iter_for_stmt, lazy_or_index_elem, lower_expr, wrap_lazy_for_of_body_close_on_throw, + LoweringContext, }; use crate::lower_patterns::*; use crate::lower_types::*; @@ -1519,16 +1520,21 @@ pub fn lower_body_stmt(ctx: &mut LoweringContext, stmt: &ast::Stmt) -> Result s, Err(_) => return undef, }; + // `f.prototype.constructor` — a *data* property (the prototype's back-pointer + // to its constructor), not a registered method, so `lookup_prototype_method` + // never finds it and the method allowlist below excludes it. When the inline + // `.prototype.constructor` read folds to this entry (no separate + // `.prototype` access ran to allocate the synthetic class id), `cid` is 0 and + // the function returned `undefined`. Route through the real prototype value — + // `js_function_prototype_value_for_read` materializes the auto-created + // prototype (whose `constructor` is `func_value`) or returns a replaced + // `f.prototype = X` — then read its `constructor` field. (Spec + // language/statements/function/S13.2_A4_*, S13.2.2_A1_*.) + if name == "constructor" { + let proto_val = js_function_prototype_value_for_read(func_value); + let jv = crate::value::JSValue::from_bits(proto_val.to_bits()); + if !jv.is_pointer() { + return undef; + } + let pptr = jv.as_pointer::(); + if pptr.is_null() { + return undef; + } + let key = crate::string::js_string_from_bytes(b"constructor".as_ptr(), 11); + let v = js_object_get_field_by_name(pptr, key as *const crate::StringHeader); + return f64::from_bits(v.bits()); + } // Look up the (already-allocated) synthetic class id for this // function value. Don't allocate one here — reads on a function // that never had any `.prototype.x = fn` assignment should