From 53a93652516971e0b478a468c96195e2a0c6fb53 Mon Sep 17 00:00:00 2001 From: Ralph Date: Tue, 23 Jun 2026 00:07:38 -0700 Subject: [PATCH 1/2] =?UTF-8?q?feat(error):=20#5247=20=E2=80=94=20source?= =?UTF-8?q?=20location=20on=20bare=20value-call=20"value=20is=20not=20a=20?= =?UTF-8?q?function"=20throw?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the `--debug-symbols` runtime source-location pipeline (#5250/#5253) to the bare value-call throw class: `f()` where `f` is not callable throws `TypeError: value is not a function` via the closure-call fallthrough's `js_closure_unbox_callee_checked` → `throw_not_callable` → `make_stack`. Codegen (`try_lower_closure_call_fallthrough`) now captures the call's source byte offset — already carried on `Expr::Call` since #5250 — before lowering the receiver/args, and replays it as `js_set_call_location` right before the throw-capable dispatch (both the ≤16 and >16 arity branches). The thrown TypeError's `.stack` then shows `at :` instead of ``, localizing nanoid/yup's `value is not a function`. Default builds are unchanged (``): the emission is a no-op unless the debug-location context installed only under `--debug-symbols` is present, and the captured offset is `0` for synthesized calls. A nested-call argument no longer shadows the location because the offset is captured before args are lowered and emitted after. Adds `issue_5247_value_call_source_location.rs` (with/without `--debug-symbols`). Per contributor guidance, no version/CHANGELOG bump (maintainer folds at merge). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/lower_call/console_promise.rs | 16 ++ .../issue_5247_value_call_source_location.rs | 149 ++++++++++++++++++ 2 files changed, 165 insertions(+) create mode 100644 crates/perry/tests/issue_5247_value_call_source_location.rs diff --git a/crates/perry-codegen/src/lower_call/console_promise.rs b/crates/perry-codegen/src/lower_call/console_promise.rs index 4bcd3a2aa8..c49134a114 100644 --- a/crates/perry-codegen/src/lower_call/console_promise.rs +++ b/crates/perry-codegen/src/lower_call/console_promise.rs @@ -937,6 +937,17 @@ pub fn try_lower_closure_call_fallthrough( // `language/{statements,expressions}/class/dstr/async-gen-meth-static-*` // (`C.method(g()).next()`). For a side-effecting receiver, evaluate it // once here and read the method off that value directly. + // + // #5247: capture this call's source byte offset NOW, before the receiver + // and arguments are lowered (a nested call in either would overwrite the + // shared pending offset). The `js_closure_unbox_callee_checked` dispatch + // below throws `TypeError: value is not a function` for a non-callable + // callee (`const f: any = 5; f()`, nanoid/yup's failure shape), and that + // throw renders `CURRENT_CALL_LOCATION` via `make_stack`. Replaying the + // offset as `js_set_call_location` right before the dispatch gives the + // throw an `at :` frame under `--debug-symbols`. `0` (and the + // default build) → no emission, unchanged `` frame. + let call_byte_offset = ctx.strings.pending_call_offset(); let prelowered_recv: Option<(String, String)> = if let Expr::PropertyGet { object, property } = callee { if receiver_must_eval_once(object.as_ref()) { @@ -1006,6 +1017,11 @@ pub fn try_lower_closure_call_fallthrough( None }; + // #5247: record the source location right before the throw-capable + // dispatch — after the receiver/args are lowered, so a nested-call + // argument's location no longer shadows this one. Applies to both arity + // branches below (the checked unbox throws in either). No-op default build. + crate::expr::calls::emit_call_location_at(ctx, call_byte_offset); let result = if lowered_args.len() <= 16 { let blk = ctx.block(); // #5504: tag-check the callee before masking to a closure pointer. diff --git a/crates/perry/tests/issue_5247_value_call_source_location.rs b/crates/perry/tests/issue_5247_value_call_source_location.rs new file mode 100644 index 0000000000..7a920fe2cf --- /dev/null +++ b/crates/perry/tests/issue_5247_value_call_source_location.rs @@ -0,0 +1,149 @@ +//! Regression test for #5247 (follow-up to #5250/#5253): the runtime +//! source-location diagnostics that #5250/#5253 gave the method-dispatch, +//! construct and ReferenceError throws are extended to the **bare value-call** +//! throw class, gated on `--debug-symbols`: +//! +//! `f()` where `f` is not callable → `TypeError: value is not a function`. +//! `const f: any = 5; f();` lowers to `Expr::Call` with a `LocalGet` callee +//! that no static dispatch claims, so codegen's closure-call fallthrough +//! (`try_lower_closure_call_fallthrough`) emits `js_closure_unbox_callee_checked`, +//! which throws via `throw_not_callable` → `make_stack`. This is the shape +//! that localizes nanoid/yup's `value is not a function`. +//! +//! Behavior: +//! • WITH `--debug-symbols`: the thrown TypeError's `.stack` contains +//! `at :` pointing at the offending call's line. +//! • WITHOUT the flag (default build): unchanged — `at `. + +use std::path::PathBuf; +use std::process::Command; +use std::sync::Once; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn workspace_root() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../..") + .canonicalize() + .expect("canonicalize workspace root") +} + +fn target_debug_dir() -> PathBuf { + std::env::var_os("CARGO_TARGET_DIR") + .map(PathBuf::from) + .unwrap_or_else(|| workspace_root().join("target")) + .join("debug") +} + +/// Build `libperry_runtime.a` once so the compiled binaries can link. +fn ensure_runtime_archive() { + static BUILD_RUNTIME: Once = Once::new(); + BUILD_RUNTIME.call_once(|| { + let cargo = std::env::var_os("CARGO").unwrap_or_else(|| "cargo".into()); + let build = Command::new(cargo) + .current_dir(workspace_root()) + .arg("build") + .arg("-p") + .arg("perry-runtime") + .output() + .expect("run cargo build -p perry-runtime"); + assert!( + build.status.success(), + "cargo build -p perry-runtime failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&build.stdout), + String::from_utf8_lossy(&build.stderr) + ); + }); +} + +fn runtime_dir() -> PathBuf { + ensure_runtime_archive(); + target_debug_dir() +} + +fn compile(root: &std::path::Path, extra_args: &[&str]) -> std::process::Output { + let entry = root.join("main.ts"); + let output = root.join("main_bin"); + let mut cmd = Command::new(perry_bin()); + cmd.current_dir(root) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .arg("--no-cache"); + for a in extra_args { + cmd.arg(a); + } + cmd.env("PERRY_NO_AUTO_OPTIMIZE", "1"); + cmd.env("PERRY_RUNTIME_DIR", runtime_dir()); + cmd.output().expect("run perry compile") +} + +fn run_fixture(fixture: &str, extra_args: &[&str]) -> String { + let dir = tempfile::tempdir().expect("tempdir"); + let root = dir.path(); + std::fs::write(root.join("main.ts"), fixture).expect("write entry"); + + let out = compile(root, extra_args); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "compile must succeed (args {extra_args:?}); stderr:\n{stderr}" + ); + + let bin = root.join("main_bin"); + let run = Command::new(&bin).output().expect("run compiled binary"); + String::from_utf8_lossy(&run.stdout).into_owned() +} + +/// `f()` is on line 4 (1 = blank from the raw-string leading newline, +/// 2 = `const`, 3 = `try {`, 4 = `f();`). +const VALUE_CALL_FIXTURE: &str = r#" +const f: any = 5; +try { + f(); +} catch (e: any) { + console.log("MSG:" + e.message); + console.log("STACK:" + e.stack); +} +"#; + +#[test] +fn debug_symbols_attaches_file_line_to_value_call_throw() { + let stdout = run_fixture(VALUE_CALL_FIXTURE, &["--debug-symbols"]); + // The non-callable value-call threw the expected TypeError. + assert!( + stdout.contains("MSG:") && stdout.contains("value is not a function"), + "expected a 'value is not a function' TypeError; got:\n{stdout}" + ); + // The stack frame names the source file and the line of `f()` (4), + // not ``. + assert!( + stdout.contains("at main.ts:4"), + "expected 'at main.ts:4' frame with --debug-symbols; got:\n{stdout}" + ); + assert!( + !stdout.contains(""), + "the location must replace the frame; got:\n{stdout}" + ); +} + +#[test] +fn default_build_keeps_anonymous_frame_for_value_call() { + let stdout = run_fixture(VALUE_CALL_FIXTURE, &[]); + assert!( + stdout.contains("value is not a function"), + "expected a 'value is not a function' TypeError; got:\n{stdout}" + ); + // Default build is unchanged: the coarse frame, no file:line. + assert!( + stdout.contains("at "), + "default build must keep the frame; got:\n{stdout}" + ); + assert!( + !stdout.contains("at main.ts:"), + "default build must NOT emit a source location; got:\n{stdout}" + ); +} From 1266fdd2bbeec130cbe2420a5f8f569e332f55bb Mon Sep 17 00:00:00 2001 From: Ralph Date: Tue, 23 Jun 2026 00:15:19 -0700 Subject: [PATCH 2/2] test(#5247): assert compiled fixture exits successfully in run_fixture CodeRabbit: guard against a crash/non-zero exit masquerading as a plain assertion failure on partial stdout. The fixtures catch the throw and `console.log` it, so the program exits cleanly. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../perry/tests/issue_5247_value_call_source_location.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/perry/tests/issue_5247_value_call_source_location.rs b/crates/perry/tests/issue_5247_value_call_source_location.rs index 7a920fe2cf..b5ec42f329 100644 --- a/crates/perry/tests/issue_5247_value_call_source_location.rs +++ b/crates/perry/tests/issue_5247_value_call_source_location.rs @@ -95,6 +95,14 @@ fn run_fixture(fixture: &str, extra_args: &[&str]) -> String { let bin = root.join("main_bin"); let run = Command::new(&bin).output().expect("run compiled binary"); + // The fixtures catch the throw and `console.log` it, so the program exits + // cleanly. Assert that, so a crash / non-zero exit can't masquerade as a + // plain assertion failure on partial stdout. + assert!( + run.status.success(), + "compiled binary must exit successfully; stderr:\n{}", + String::from_utf8_lossy(&run.stderr) + ); String::from_utf8_lossy(&run.stdout).into_owned() }