fix(hir): clear callee marker when lowering short-circuit/conditional operands by proggeramlug · Pull Request #5784 · PerryTS/perry

proggeramlug · 2026-06-29T00:00:08Z

Problem

Calling a native builtin reached through a short-circuit or conditional expression threw TypeError: value is not a function when the builtin branch was taken:

(K || JSON.parse)(x)      // K falsy  → throws
(K ?? JSON.parse)(x)      // K nullish → throws
(c ? a : JSON.parse)(x)   // c falsy  → throws
(0, JSON.parse)(x)        // comma     → throws

A user function via ||, a builtin stored-then-called (let g = K || JSON.parse; g(x)), and a direct intrinsic call (JSON.parse(x)) all worked — only the inline short-circuit/conditional callee broke.

Root cause (perry-hir)

Lowering a call sets ctx.lowering_call_callee = true before lowering the callee, so a direct intrinsic call (JSON.parse(x), Date.now()) takes the fast path: the member-tail reroute-undo collapses the receiver to a bare GlobalGet(0) and codegen emits the intrinsic directly.

But a binary/logical/conditional/sequence expression in callee position is itself the callee — its operands/branches are values, not the immediate callee member. The marker leaked through lower_bin_expr / lower_cond / lower_seq into those operands, so a nested builtin-namespace member (JSON.parse) was collapsed to the value-less intrinsic form PropertyGet { GlobalGet(0), "parse" } (the namespace name dropped). That form has no value materialization and lowers to undefined — so the short-circuit/conditional result threw when called.

Verified via --trace hir: the buggy ||-callee form was PropertyGet { GlobalGet(0), "parse" }; the working let-init form was PropertyGet { PropertyGet { GlobalGet(0), "JSON" }, "parse" }. Routing the collapsed form by property name alone would be unsound (JSON.parse and Date.parse both collapse to …, "parse"), so the fix is to not collapse it.

Fix

Save/clear/restore lowering_call_callee around lowering the operands in lower_bin_expr (||/??/&&/all binary ops), lower_cond (ternary), and lower_seq (comma). A nested call operand re-sets the flag for its own callee, so direct intrinsic calls keep their fast path.

This is the runtime shape behind axios's transformRequest ((K || JSON.parse)(data)) and other (x || nativeBuiltin)() sites.

Test

crates/perry/tests/short_circuit_builtin_callee.rs (run under a wall-clock timeout): native builtin via || / ?? / && / ternary / comma callees, plus user-fn-via-||, stored-then-called, and direct JSON.parse(x) (all must keep working). cargo test -p perry-hir and -p perry-codegen green.

Summary by CodeRabbit

Bug Fixes
- Fixed short-circuit, conditional, comma, and binary expression lowering so nested call targets are handled correctly.
- Prevented built-in callees like JSON.parse and JSON.stringify from being misread as non-callable values in these expression forms.
Tests
- Added a regression test covering direct calls, stored calls, and short-circuit/conditional callee patterns to verify expected runtime behavior.

… operands Calling a native builtin reached through a short-circuit or conditional expression — `(K || JSON.parse)(x)`, `(K ?? B.method)(x)`, `(c ? a : JSON.parse)(x)`, `(0, JSON.parse)(x)` — threw `TypeError: value is not a function` when the builtin branch was taken. Root cause (perry-hir): lowering a call sets `ctx.lowering_call_callee` before lowering the callee so a DIRECT intrinsic call (`JSON.parse(x)`, `Date.now()`) takes the fast path — the member-tail reroute-undo collapses the receiver to a bare `GlobalGet(0)` and codegen emits the intrinsic. But a binary/logical/ conditional/sequence expression in callee position is ITSELF the callee; its operands/branches are values, not the immediate callee member. The marker leaked through `lower_bin_expr` / `lower_cond` / `lower_seq` into those operands, so a nested builtin-namespace member (`JSON.parse`) was collapsed to the value-less intrinsic form `PropertyGet { GlobalGet(0), "parse" }` (the namespace name dropped). That form has no value materialization and lowers to `undefined`, so the short-circuit/conditional result threw when called. Stored-then-called (`let g = K || JSON.parse; g(x)`) and direct calls were unaffected because the marker was correctly false / true there. Fix: save/clear/restore `lowering_call_callee` around lowering the operands in `lower_bin_expr` (`||`/`??`/`&&`/all binary), `lower_cond` (ternary), and `lower_seq` (comma). A nested *call* operand re-sets the flag for its own callee, so direct intrinsic calls keep their fast path. This is the runtime shape behind axios's `transformRequest` (`(K || JSON.parse)`) and other `(x || nativeBuiltin)()` call sites. Adds an integration test covering `||`/`??`/`&&`/ternary/comma callees with native builtins, plus user-fn-via-`||`, stored-then-called, and direct-call (must keep working), run under a wall-clock timeout.

coderabbitai · 2026-06-29T00:00:30Z

📝 Walkthrough

Walkthrough

Three lowering functions (lower_bin_expr, lower_cond, lower_seq) now save ctx.lowering_call_callee, set it to false while lowering operands/branches/elements, and restore it afterward. A regression test is added that compiles and runs TypeScript programs exercising JSON builtins as callees through short-circuit, ternary, and comma expressions.

Fix lowering_call_callee flag leak

Layer / File(s)	Summary
Save/restore `lowering_call_callee` in binary, conditional, and sequence lowering `crates/perry-hir/src/lower/lower_expr/arm_bin.rs`, `crates/perry-hir/src/lower/expr_misc.rs`	`lower_bin_expr`, `lower_cond`, and `lower_seq` each save `ctx.lowering_call_callee`, set it to `false` before lowering operands/branches/elements, and restore the saved value afterward. `lower_seq` also refactors the element-lowering loop to `map(...).collect()`.
Regression test for builtin callees in short-circuit callee position `crates/perry/tests/short_circuit_builtin_callee.rs`	Adds `compile_and_run` infrastructure (binary path helper, compiler invocation, 30s timeout, stdout capture) and a test that exercises `JSON.parse`/`JSON.stringify` through `\|\|`, `??`, `&&`, ternary, comma, and stored-then-called callee forms, asserting expected output for each.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 The flag was leaking, oh what a mess,
Through ternaries and commas it caused distress.
Now save, set false, restore in line—
The callee knows its place just fine.
Each builtin parsed without a care! 🎉

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The writeup is detailed but doesn't follow the required template and omits Summary, Changes, Related issue, and Test plan sections.	Rewrite the PR description using the required headings: Summary, Changes, Related issue, Test plan, Screenshots/output, and Checklist.
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the main fix: clearing the callee marker while lowering short-circuit and conditional operands.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/perry-hir/src/lower/expr_misc.rs`:
- Around line 42-50: The conditional lowering in lower_expr for
Expr::Conditional is eagerly evaluating cond.test, cond.cons, and cond.alt
before any error is handled, so it no longer short-circuits on the first
failure. Refactor this block to lower cond.test first, return immediately on
error, and only then lower cond.cons and cond.alt in sequence before restoring
ctx.lowering_call_callee. Use the lower_expr and Expr::Conditional symbols to
keep the change localized and preserve the existing lowering-state behavior.

In `@crates/perry-hir/src/lower/lower_expr/arm_bin.rs`:
- Around line 179-185: In lower_expr_bin, restore the original short-circuit
behavior so bin.right is not lowered after bin.left fails; lower the left side
first, immediately propagate its error with ?, and only then lower the right
side. Keep the lowering_call_callee save/restore around the same block in
arm_bin.rs, but make sure the state is reset before returning on error so
lower_expr does not continue mutating context after a failure.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3cd365d1-070b-4cf6-ae5d-4fd969cdd8ea

📥 Commits

Reviewing files that changed from the base of the PR and between ffb0d6a and e706046.

📒 Files selected for processing (3)

crates/perry-hir/src/lower/expr_misc.rs
crates/perry-hir/src/lower/lower_expr/arm_bin.rs
crates/perry/tests/short_circuit_builtin_callee.rs

coderabbitai · 2026-06-29T00:05:03Z

+    let prev_call_callee = ctx.lowering_call_callee;
+    ctx.lowering_call_callee = false;
+    let condition = lower_expr(ctx, &cond.test);
+    let then_expr = lower_expr(ctx, &cond.cons);
+    let else_expr = lower_expr(ctx, &cond.alt);
+    ctx.lowering_call_callee = prev_call_callee;
    Ok(Expr::Conditional {
-        condition,
-        then_expr,
-        else_expr,
+        condition: Box::new(condition?),
+        then_expr: Box::new(then_expr?),


🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep conditional lowering short-circuiting on the first error.

This eagerly lowers all three subexpressions before any ?, so cond.cons and cond.alt still run even if cond.test already failed. That changes the previous error path and can mutate lowering state after the first failure.

Proposed fix

let prev_call_callee = ctx.lowering_call_callee; ctx.lowering_call_callee = false; - let condition = lower_expr(ctx, &cond.test); - let then_expr = lower_expr(ctx, &cond.cons); - let else_expr = lower_expr(ctx, &cond.alt); + let lowered = (|| -> Result<(Box<Expr>, Box<Expr>, Box<Expr>)> { + Ok(( + Box::new(lower_expr(ctx, &cond.test)?), + Box::new(lower_expr(ctx, &cond.cons)?), + Box::new(lower_expr(ctx, &cond.alt)?), + )) + })(); ctx.lowering_call_callee = prev_call_callee; + let (condition, then_expr, else_expr) = lowered?; Ok(Expr::Conditional { - condition: Box::new(condition?), - then_expr: Box::new(then_expr?), - else_expr: Box::new(else_expr?), + condition, + then_expr, + else_expr, })

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

let prev_call_callee = ctx.lowering_call_callee;

ctx.lowering_call_callee = false;

let condition = lower_expr(ctx, &cond.test);

let then_expr = lower_expr(ctx, &cond.cons);

let else_expr = lower_expr(ctx, &cond.alt);

ctx.lowering_call_callee = prev_call_callee;

Ok(Expr::Conditional {

condition,

then_expr,

else_expr,

condition: Box::new(condition?),

then_expr: Box::new(then_expr?),

let prev_call_callee = ctx.lowering_call_callee;

ctx.lowering_call_callee = false;

let lowered = (|| -> Result<(Box<Expr>, Box<Expr>, Box<Expr>)> {

Ok((

Box::new(lower_expr(ctx, &cond.test)?),

Box::new(lower_expr(ctx, &cond.cons)?),

Box::new(lower_expr(ctx, &cond.alt)?),

))

})();

ctx.lowering_call_callee = prev_call_callee;

let (condition, then_expr, else_expr) = lowered?;

Ok(Expr::Conditional {

condition,

then_expr,

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-hir/src/lower/expr_misc.rs` around lines 42 - 50, The conditional lowering in lower_expr for Expr::Conditional is eagerly evaluating cond.test, cond.cons, and cond.alt before any error is handled, so it no longer short-circuits on the first failure. Refactor this block to lower cond.test first, return immediately on error, and only then lower cond.cons and cond.alt in sequence before restoring ctx.lowering_call_callee. Use the lower_expr and Expr::Conditional symbols to keep the change localized and preserve the existing lowering-state behavior.

coderabbitai · 2026-06-29T00:05:03Z

+    let prev_call_callee = ctx.lowering_call_callee;
+    ctx.lowering_call_callee = false;
+    let left = lower_expr(ctx, &bin.left);
+    let right = lower_expr(ctx, &bin.right);
+    ctx.lowering_call_callee = prev_call_callee;
+    let left = Box::new(left?);
+    let right = Box::new(right?);


🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve the original short-circuiting on lowering errors.

This now lowers bin.right even when bin.left has already failed. lower_expr is stateful, so doing extra lowering work after the first error can perturb context/diagnostics before the error is returned. Please restore the flag without losing the old ? behavior.

Proposed fix

let prev_call_callee = ctx.lowering_call_callee; ctx.lowering_call_callee = false; - let left = lower_expr(ctx, &bin.left); - let right = lower_expr(ctx, &bin.right); + let lowered = (|| -> Result<(Box<Expr>, Box<Expr>)> { + Ok(( + Box::new(lower_expr(ctx, &bin.left)?), + Box::new(lower_expr(ctx, &bin.right)?), + )) + })(); ctx.lowering_call_callee = prev_call_callee; - let left = Box::new(left?); - let right = Box::new(right?); + let (left, right) = lowered?;

[ source_other ]

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

let prev_call_callee = ctx.lowering_call_callee;

ctx.lowering_call_callee = false;

let left = lower_expr(ctx, &bin.left);

let right = lower_expr(ctx, &bin.right);

ctx.lowering_call_callee = prev_call_callee;

let left = Box::new(left?);

let right = Box::new(right?);

let prev_call_callee = ctx.lowering_call_callee;

ctx.lowering_call_callee = false;

let lowered = (|| -> Result<(Box<Expr>, Box<Expr>)> {

Ok((

Box::new(lower_expr(ctx, &bin.left)?),

Box::new(lower_expr(ctx, &bin.right)?),

))

})();

ctx.lowering_call_callee = prev_call_callee;

let (left, right) = lowered?;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-hir/src/lower/lower_expr/arm_bin.rs` around lines 179 - 185, In lower_expr_bin, restore the original short-circuit behavior so bin.right is not lowered after bin.left fails; lower the left side first, immediately propagate its error with ?, and only then lower the right side. Keep the lowering_call_callee save/restore around the same block in arm_bin.rs, but make sure the state is reset before returning on error so lower_expr does not continue mutating context after a failure.

proggeramlug · 2026-06-29T00:15:59Z

Lands a subcluster of #5591 (built-ins tail): native builtin reached through a short-circuit/conditional/comma callee was thrown as 'value is not a function'.

coderabbitai Bot reviewed Jun 29, 2026

View reviewed changes

proggeramlug merged commit 9107f91 into PerryTS:main Jun 29, 2026
15 checks passed

dlee mentioned this pull request Jul 10, 2026

Self-recursive function with a ternary miscompiles: i64 specialization has empty body (ret i64 0) #6221

Closed

Navigation Menu

Uh oh!

Uh oh!

fix(hir): clear callee marker when lowering short-circuit/conditional operands - #5784

fix(hir): clear callee marker when lowering short-circuit/conditional operands#5784
proggeramlug merged 1 commit into
PerryTS:mainfrom
proggeramlug:fix-short-circuit-builtin-callee

proggeramlug commented Jun 29, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 29, 2026 •

edited

Loading

Walkthrough

Estimated code review effort

Poem

❌ Failed checks (2 warnings)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Jun 29, 2026

Uh oh!

coderabbitai Bot Jun 29, 2026

Uh oh!

proggeramlug commented Jun 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

proggeramlug commented Jun 29, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Root cause (perry-hir)

Fix

Test

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Poem

❌ Failed checks (2 warnings)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

proggeramlug commented Jun 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

proggeramlug commented Jun 29, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 29, 2026 •

edited

Loading