diff --git a/CLAUDE.md b/CLAUDE.md index bb1a58d2ce..d1aea30fca 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co Perry is a native TypeScript compiler written in Rust that compiles TypeScript source code directly to native executables. It uses SWC for TypeScript parsing and LLVM for code generation. -**Current Version:** 0.5.1335 +**Current Version:** 0.5.1336 ## TypeScript Parity Status diff --git a/Cargo.lock b/Cargo.lock index 9cad5898d6..bde5416496 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5547,7 +5547,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "anyhow", "base64", @@ -5607,14 +5607,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "cc", "libc", @@ -5622,7 +5622,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "anyhow", "inkwell", @@ -5639,7 +5639,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "anyhow", "perry-hir", @@ -5647,7 +5647,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "anyhow", "perry-hir", @@ -5655,7 +5655,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "anyhow", "perry-dispatch", @@ -5664,7 +5664,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "anyhow", "perry-hir", @@ -5672,7 +5672,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "anyhow", "base64", @@ -5684,7 +5684,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "anyhow", "perry-hir", @@ -5692,7 +5692,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "anyhow", "async-trait", @@ -5721,14 +5721,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "serde", "serde_json", @@ -5736,7 +5736,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1335" +version = "0.5.1336" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5747,7 +5747,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "anyhow", "clap", @@ -5762,7 +5762,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "block2", "objc2", @@ -5772,7 +5772,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "argon2", "perry-ffi", @@ -5780,7 +5780,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ffi", "reqwest", @@ -5789,7 +5789,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "bcrypt", "perry-ffi", @@ -5797,7 +5797,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ffi", "rusqlite", @@ -5805,7 +5805,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ffi", "scraper", @@ -5813,7 +5813,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ffi", "perry-runtime", @@ -5821,7 +5821,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "chrono", "cron", @@ -5831,7 +5831,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "chrono", "perry-ffi", @@ -5839,7 +5839,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ffi", "rust_decimal", @@ -5847,7 +5847,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ffi", "serde_json", @@ -5855,7 +5855,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5863,7 +5863,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ffi", "perry-runtime", @@ -5871,14 +5871,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "bytes", "http-body-util", @@ -5896,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "bytes", "lazy_static", @@ -5909,7 +5909,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "bytes", "h2", @@ -5933,7 +5933,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "lazy_static", "perry-ffi", @@ -5943,7 +5943,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "base64", "jsonwebtoken", @@ -5954,7 +5954,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "lru", "perry-ffi", @@ -5963,7 +5963,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "chrono", "perry-ffi", @@ -5971,7 +5971,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "bson", "futures-util", @@ -5983,7 +5983,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "chrono", "perry-ffi", @@ -5993,7 +5993,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "nanoid", "perry-ffi", @@ -6002,7 +6002,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "bytes", "perry-ffi", @@ -6015,7 +6015,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -6034,7 +6034,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "lettre", "perry-ffi", @@ -6044,7 +6044,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ffi", "printpdf", @@ -6052,7 +6052,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ffi", "sqlx", @@ -6061,7 +6061,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "governor", "perry-ffi", @@ -6069,7 +6069,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "fast_image_resize", "image", @@ -6079,14 +6079,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "lazy_static", "perry-ffi", @@ -6095,7 +6095,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ffi", "perry-runtime", @@ -6104,7 +6104,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ffi", "uuid", @@ -6112,7 +6112,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ffi", "regex", @@ -6122,7 +6122,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "futures-util", "lazy_static", @@ -6135,7 +6135,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "brotli", "flate2", @@ -6145,7 +6145,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "dashmap", "once_cell", @@ -6154,7 +6154,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "anyhow", "perry-api-manifest", @@ -6172,7 +6172,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "anyhow", "perry-diagnostics", @@ -6184,7 +6184,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "anyhow", "base64", @@ -6226,14 +6226,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6328,14 +6328,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "anyhow", "perry-hir", @@ -6344,14 +6344,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "base64", "itoa", @@ -6368,7 +6368,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "rand 0.10.1", "serde", @@ -6378,7 +6378,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6401,7 +6401,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "base64", "block2", @@ -6417,7 +6417,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "base64", "block2", @@ -6432,7 +6432,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1335" +version = "0.5.1336" [[package]] name = "perry-ui-test" @@ -6443,11 +6443,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1335" +version = "0.5.1336" [[package]] name = "perry-ui-tvos" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "base64", "block2", @@ -6463,7 +6463,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "base64", "block2", @@ -6479,7 +6479,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "block2", "libc", @@ -6492,7 +6492,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "base64", "libc", @@ -6509,14 +6509,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "anyhow", "base64", @@ -6532,7 +6532,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1335" +version = "0.5.1336" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 1d85cb9385..db6247aa3b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1335" +version = "0.5.1336" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7591-discard-expr-value-leak.md b/changelog.d/7591-discard-expr-value-leak.md new file mode 100644 index 0000000000..923c75b801 --- /dev/null +++ b/changelog.d/7591-discard-expr-value-leak.md @@ -0,0 +1,52 @@ +**A typed-array element store used as an expression no longer evaluates to `0` +(#7590).** + +```ts +const buf = new Uint8Array(4); +sink((buf[0] = 5), 5); // was 0, now 5 +n = buf[1] = 7; // was 0, now 7 +sink((buf[2] = 3) + 100, 103); // was 100, now 103 +``` + +The stores themselves always landed correctly — only the expression's *value* +was wrong, so this was silent: a wrong number, no crash, no diagnostic. An +assignment expression must evaluate to the assigned value (ES2024 §13.15.2). + +`ctx.discard_expr_value` means **"this STATEMENT's value is discarded"**. It is +set once per `Stmt::Expr` and `lower_expr` never cleared it while recursing — +the only reset in the tree is for constructor arguments +(`lower_call/new_ctor_args.rs`). Four sites read it as though it meant "this +EXPRESSION's value is discarded" and returned `double_literal(0.0)`, so they +fired while the store was an *operand* of the statement: +`expr/index_set.rs` (typed-array store, proven-view checked store) and +`expr/arrays_finds.rs` (`Uint8ArraySet` / buffer stores). `expr/dispatch.rs` +also reads the flag, but only to choose a materialization path, and is +unaffected. + +The fix adds `FnCtx::discard_this_expr`, which `dispatch::lower_expr` +**takes** (`mem::take`) at the top of every dispatch. It therefore reaches +exactly one expression — the one the statement is made of — and every operand +lowered beneath it reads `false`. The handlers that need the answer receive it +as a parameter rather than reading the field, because they consult it *after* +lowering their operands, by which point the field has been taken again; reading +a field there would have reintroduced the same bug in a subtler form. + +Found while trying to gate `arr.push(x)`'s length computation on the same flag +as a performance change: `js_array_length` is not a field read (it resolves +Proxy arrays through the `get` trap and probes the registered-Set/Map side +tables) and a statement-position push discards its result, which is 8–13% of +`push_cls` (see #7511). That optimisation produced this exact bug for +`sink(a.push(10))`, `n = a.push(20)`, `a.push(1) + 100` and +`a.push(1) > 0 ? 7 : 9`, which is what exposed the pre-existing one. The +optimisation itself is **not** included here — it wants this fix first, and +then the same non-leaking signal. + +Regression test `test-files/test_typed_array_store_expression_value.ts` consumes +a store's value in call-argument, assignment, arithmetic, conditional and +nested-store position, and keeps two discarded stores to prove the ordinary +path still writes. That combination matters: the discarded form kept working +throughout, so a "does it still run" smoke test passes while the bug is live. + +Verified against 78 `test-files` programs (typed-array/buffer weighted) — the +only behavioural difference is this test. `perry-codegen`'s failure set is +unchanged from `origin/main`. diff --git a/crates/perry-codegen/src/codegen/closure.rs b/crates/perry-codegen/src/codegen/closure.rs index 3d893aa969..3f6c55ce17 100644 --- a/crates/perry-codegen/src/codegen/closure.rs +++ b/crates/perry-codegen/src/codegen/closure.rs @@ -891,6 +891,7 @@ pub(super) fn compile_closure( const_number_locals: std::collections::HashMap::new(), current_block: 0, discard_expr_value: false, + discard_this_expr: false, func_names, strings, loop_targets: Vec::new(), diff --git a/crates/perry-codegen/src/codegen/entry.rs b/crates/perry-codegen/src/codegen/entry.rs index 78f3824364..c67e7d8f02 100644 --- a/crates/perry-codegen/src/codegen/entry.rs +++ b/crates/perry-codegen/src/codegen/entry.rs @@ -707,6 +707,7 @@ pub(super) fn compile_module_entry( const_number_locals: HashMap::new(), current_block: 0, discard_expr_value: false, + discard_this_expr: false, func_names, strings, loop_targets: Vec::new(), @@ -1372,6 +1373,7 @@ pub(super) fn compile_module_entry( const_number_locals: HashMap::new(), current_block: 0, discard_expr_value: false, + discard_this_expr: false, func_names, strings, loop_targets: Vec::new(), diff --git a/crates/perry-codegen/src/codegen/function.rs b/crates/perry-codegen/src/codegen/function.rs index 45dddb8a4b..855c6b5da8 100644 --- a/crates/perry-codegen/src/codegen/function.rs +++ b/crates/perry-codegen/src/codegen/function.rs @@ -694,6 +694,7 @@ pub(super) fn compile_function( const_number_locals: std::collections::HashMap::new(), current_block: 0, discard_expr_value: false, + discard_this_expr: false, func_names, strings, loop_targets: Vec::new(), diff --git a/crates/perry-codegen/src/codegen/method.rs b/crates/perry-codegen/src/codegen/method.rs index f759cb6688..c384174272 100644 --- a/crates/perry-codegen/src/codegen/method.rs +++ b/crates/perry-codegen/src/codegen/method.rs @@ -429,6 +429,7 @@ pub(super) fn compile_method( const_number_locals: std::collections::HashMap::new(), current_block: 0, discard_expr_value: false, + discard_this_expr: false, func_names, strings, loop_targets: Vec::new(), @@ -1484,6 +1485,7 @@ pub(super) fn compile_static_method( const_number_locals: std::collections::HashMap::new(), current_block: 0, discard_expr_value: false, + discard_this_expr: false, func_names, strings, loop_targets: Vec::new(), diff --git a/crates/perry-codegen/src/expr/arrays_finds.rs b/crates/perry-codegen/src/expr/arrays_finds.rs index 8e8dd8a43a..43a7dd572b 100644 --- a/crates/perry-codegen/src/expr/arrays_finds.rs +++ b/crates/perry-codegen/src/expr/arrays_finds.rs @@ -217,7 +217,12 @@ pub(crate) fn lower_buffer_index_get_i32( Ok(slow) } -pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { +pub(crate) fn lower( + ctx: &mut FnCtx<'_>, + expr: &Expr, + // #7590: THIS expression's value is discarded (not merely the statement's). + value_discarded: bool, +) -> Result { match expr { Expr::BoxedPrimitiveNew { kind, @@ -785,7 +790,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { if let Some(store) = lower_buffer_store(ctx, array, index, value, BufferAccessSpec::uint8array_set())? { - if ctx.discard_expr_value { + if value_discarded { return Ok(double_literal(0.0)); } return Ok(materialize_js_value( @@ -821,7 +826,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { &[(I64, &handle), (DOUBLE, &key), (DOUBLE, &val)], ) }; - if ctx.discard_expr_value { + if value_discarded { return Ok(double_literal(0.0)); } return Ok(result); @@ -892,7 +897,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { false, Vec::new(), ); - if ctx.discard_expr_value { + if value_discarded { return Ok(double_literal(0.0)); } Ok(materialize_js_value(ctx, slow, reason)) @@ -909,7 +914,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { value, BufferAccessSpec::buffer_index_set(), )? { - if ctx.discard_expr_value { + if value_discarded { return Ok(double_literal(0.0)); } return Ok(materialize_js_value( @@ -982,7 +987,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { false, Vec::new(), ); - if ctx.discard_expr_value { + if value_discarded { return Ok(double_literal(0.0)); } Ok(materialize_js_value(ctx, slow, reason)) diff --git a/crates/perry-codegen/src/expr/dispatch.rs b/crates/perry-codegen/src/expr/dispatch.rs index b2114556fb..401f0686e5 100644 --- a/crates/perry-codegen/src/expr/dispatch.rs +++ b/crates/perry-codegen/src/expr/dispatch.rs @@ -21,6 +21,13 @@ use crate::types::DOUBLE; /// here is a dispatch table; each module's `lower(ctx, expr)` contains the /// original arm bodies verbatim. pub(crate) fn lower_expr(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { + // #7590: TAKE the "this expression's value is discarded" flag before doing + // anything else. `lower_stmt` set it for the statement's own expression; + // taking it here means every operand lowered below reads `false`, so a + // consumed store (`sink(buf[0] = 5);`) is never mistaken for a discarded + // one. Handlers that care receive it as an argument, because they consult + // it after lowering their operands — by which point the field is gone. + let value_discarded = std::mem::take(&mut ctx.discard_this_expr); if let Some(lowered) = lower_expr_value(ctx, expr)? { if ctx.discard_expr_value { return Ok(materialize_js_value_without_record(ctx, lowered)); @@ -52,7 +59,7 @@ pub(crate) fn lower_expr(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { super::objects_arrays_lit::lower(ctx, expr) } Expr::IndexGet { .. } => super::index_get::lower(ctx, expr), - Expr::IndexSet { .. } => super::index_set::lower(ctx, expr), + Expr::IndexSet { .. } => super::index_set::lower(ctx, expr, value_discarded), Expr::PropertySet { .. } => super::property_set::lower(ctx, expr), Expr::PropertyGet { .. } => super::property_get::lower(ctx, expr), Expr::Conditional { .. } => super::conditional::lower(ctx, expr), @@ -233,7 +240,7 @@ pub(crate) fn lower_expr(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { | Expr::ArrayEntries(..) | Expr::ArrayKeys(..) | Expr::ArrayValues(..) - | Expr::ClassRef(..) => super::arrays_finds::lower(ctx, expr), + | Expr::ClassRef(..) => super::arrays_finds::lower(ctx, expr, value_discarded), Expr::NativeMemoryFillU32 { .. } | Expr::NativeMemoryCopy { .. } => { super::native_memory::lower(ctx, expr) } diff --git a/crates/perry-codegen/src/expr/index_set.rs b/crates/perry-codegen/src/expr/index_set.rs index a25ca75abb..7993835798 100644 --- a/crates/perry-codegen/src/expr/index_set.rs +++ b/crates/perry-codegen/src/expr/index_set.rs @@ -683,7 +683,12 @@ fn lower_packed_numeric_loop_index_set( Ok(val_double) } -pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { +pub(crate) fn lower( + ctx: &mut FnCtx<'_>, + expr: &Expr, + // #7590: THIS expression's value is discarded (not merely the statement's). + value_discarded: bool, +) -> Result { match expr { Expr::IndexSet { object, @@ -751,7 +756,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { )); } if let Some(store) = lower_typed_array_store(ctx, object, index, value)? { - if ctx.discard_expr_value { + if value_discarded { return Ok(double_literal(0.0)); } return Ok(materialize_js_value( @@ -766,7 +771,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { if let Some(stored) = super::try_lower_proven_view_checked_store(ctx, object, index, value)? { - if ctx.discard_expr_value { + if value_discarded { return Ok(double_literal(0.0)); } return Ok(materialize_js_value( @@ -845,7 +850,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { value, BufferAccessSpec::uint8array_set(), )? { - if ctx.discard_expr_value { + if value_discarded { return Ok(double_literal(0.0)); } return Ok(materialize_js_value( diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index 42bd4e681a..c494d3f3cf 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -229,6 +229,26 @@ pub(crate) struct FnCtx<'a> { /// True while lowering an expression statement whose resulting JS value /// will be discarded. pub discard_expr_value: bool, + + /// #7590: is the expression **currently being dispatched** one whose value + /// is discarded — as opposed to [`Self::discard_expr_value`], which says + /// only that the enclosing STATEMENT's value is discarded? + /// + /// The two are not the same, and reading the wrong one is a silent + /// wrong-value bug. `discard_expr_value` is set once per `Stmt::Expr` and + /// is never cleared as `lower_expr` recurses, so it is still set while + /// lowering the operands of `sink(buf[0] = 5);` — where the store's value + /// is very much consumed. Four sites read it as if it meant this field and + /// returned `0.0`, making a typed-array store used as an expression + /// evaluate to `0` instead of the assigned value (ES2024 §13.15.2). + /// + /// This one is **taken** (`mem::take`) at the top of + /// [`dispatch::lower_expr`], so it reaches exactly one expression — the one + /// the statement is made of — and every operand lowered beneath it reads + /// `false`. Handlers that need it receive it as a parameter rather than + /// reading the field, because they consult it *after* lowering their + /// operands, by which point the field has been taken again. + pub discard_this_expr: bool, /// HIR FuncId → LLVM function name. Resolved at the top of /// `compile_module` so `FuncRef(id)` calls know what to emit. pub func_names: &'a std::collections::HashMap, diff --git a/crates/perry-codegen/src/expr/proxy_reflect.rs b/crates/perry-codegen/src/expr/proxy_reflect.rs index cadb930131..d31fb9ac33 100644 --- a/crates/perry-codegen/src/expr/proxy_reflect.rs +++ b/crates/perry-codegen/src/expr/proxy_reflect.rs @@ -1270,6 +1270,10 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { index: key.clone(), value: value.clone(), }, + // #7590: a `PutValue` routed through the index-set fast + // path returns the assigned value to ITS caller, which may + // well consume it. Never the discarded form. + false, ); } if let Some(result) = diff --git a/crates/perry-codegen/src/stmt/mod.rs b/crates/perry-codegen/src/stmt/mod.rs index a574e8cdf3..234514f238 100644 --- a/crates/perry-codegen/src/stmt/mod.rs +++ b/crates/perry-codegen/src/stmt/mod.rs @@ -260,7 +260,12 @@ pub(crate) fn lower_stmt(ctx: &mut FnCtx<'_>, stmt: &Stmt) -> Result<()> { Stmt::Expr(e) => { let prev_discard = ctx.discard_expr_value; ctx.discard_expr_value = true; + // #7590: the non-leaking companion. `lower_expr` takes this at the + // top of its dispatch, so only `e` itself sees it — an operand of + // `e` (`sink(a.push(10))`) reads `false` and keeps its value. + ctx.discard_this_expr = true; let result = lower_expr(ctx, e); + ctx.discard_this_expr = false; ctx.discard_expr_value = prev_discard; let _ = result?; Ok(()) diff --git a/test-files/test_typed_array_store_expression_value.ts b/test-files/test_typed_array_store_expression_value.ts new file mode 100644 index 0000000000..483744b863 --- /dev/null +++ b/test-files/test_typed_array_store_expression_value.ts @@ -0,0 +1,34 @@ +// #7590: a typed-array element store used as an EXPRESSION must evaluate to +// the assigned value (ES2024 §13.15.2), not 0. +// +// The bug was that `ctx.discard_expr_value` — "this STATEMENT's value is +// discarded" — was not cleared as `lower_expr` recursed, so the store lowering +// saw it set while it was an OPERAND and returned 0.0. Every line below is an +// expression statement, so the flag is set, but the store's value is consumed +// in each; the discarded forms at the end must keep working too. +const buf = new Uint8Array(8); +const out: string[] = []; + +function check(label: string, got: number, want: number): void { + out.push(got === want ? label + ":ok" : label + ":WRONG got=" + got + " want=" + want); +} + +// consumed as a call argument +check("arg", (buf[0] = 5), 5); +// consumed by an assignment +let n = 0; +n = buf[1] = 7; +check("assign", n, 7); +// consumed by arithmetic +check("binary", (buf[2] = 3) + 100, 103); +// consumed by a condition +check("ternary", (buf[3] = 1) > 0 ? 7 : 9, 7); +// consumed through a nested store +check("nested", (buf[4] = (buf[5] = 2)), 2); + +// the ordinary discarded forms still store correctly +buf[6] = 11; +buf[7] = 12; + +console.log(out.join(" ")); +console.log("buf", buf[0], buf[1], buf[2], buf[3], buf[4], buf[5], buf[6], buf[7]);