Skip to content

jit: fix recursive call return value type tagging - #8499

Draft
kyokuping wants to merge 1 commit into
RustPython:mainfrom
kyokuping:codex/jit-recursive-return-type
Draft

jit: fix recursive call return value type tagging#8499
kyokuping wants to merge 1 commit into
RustPython:mainfrom
kyokuping:codex/jit-recursive-return-type

Conversation

@kyokuping

@kyokuping kyokuping commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Assisted-by: Codex:5.6-sol

Summary

Recursive call results were previously hardcoded as JitValue::Int(...). Even when Cranelift gave back a value with the right machine type, the JIT still wrote it down as an integer at compile time.

As a result, recursive functions returning bool or float were incorrectly typed. Functions with no return value could also try to read a return slot that was not there.

  • Use slice pattern matching on return slots so we don't index into a nonexistent result
  • Match the declared return type (self.sig.ret) with the actual return slot to preserve bool, float, and None return types
  • Return NotSupported when the declared return type and actual return slot do not match
  • Add regression tests for recursive functions returning bool and float
    • recursive_float: verifies that a recursive call result remains tagged as Float, allowing the following / 2.0 operation to consume the underlying F64 value correctly
    • recursive_bool: verifies that a recursive call result remains tagged as Bool (I8), allowing the following not operation to compile and execute correctly

Summary by CodeRabbit

  • Bug Fixes

    • Recursive JIT calls now correctly handle functions returning no value, integers, booleans, and floating-point values.
    • Unsupported return shapes are reported clearly instead of being interpreted incorrectly.
  • Tests

    • Added coverage for recursive boolean evaluation, including negation and base cases.
    • Added coverage for recursive floating-point calculations across multiple recursion levels.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Recursive JIT calls now derive results from the compiled function return signature. The change supports no-result and single-result calls, rejects unsupported result shapes, and adds boolean and floating-point recursion tests.

Changes

Recursive JIT result handling

Layer / File(s) Summary
Return-signature-based recursive calls
crates/jit/src/instructions.rs
Recursive calls now support JitValue::None for no-result calls and convert single results using self.sig.ret. Multiple or incompatible results return JitCompileError::NotSupported.
Recursive boolean and float coverage
crates/jit/tests/bool_tests.rs, crates/jit/tests/float_tests.rs
Tests cover recursive boolean negation and recursive floating-point halving at multiple recursion depths.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant RecursiveJITCall
  participant CompiledFunction
  participant JitValue
  RecursiveJITCall->>CompiledFunction: execute recursive call
  CompiledFunction-->>RecursiveJITCall: return result shape and value
  RecursiveJITCall->>JitValue: convert value using self.sig.ret
Loading

Possibly related PRs

Suggested reviewers: shaharnaveh

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the fix for recursive JIT call return-value type tagging.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

@kyokuping
kyokuping marked this pull request as draft August 11, 2026 21:44

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
crates/jit/src/instructions.rs (1)

561-564: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add recursive None coverage.

Line 562 adds the no-result recursive-call path. The added tests cover only Bool and Float. Add a recursive function declared with None that reaches return recursive_none(n - 1).

🤖 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/jit/src/instructions.rs` around lines 561 - 564, Extend the
recursive-call test coverage around the return handling in the JIT instruction
flow to include a function declared with return type None that executes return
recursive_none(n - 1). Verify this path compiles and returns JitValue::None,
preserving the existing Bool and Float recursive cases.
🤖 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.

Nitpick comments:
In `@crates/jit/src/instructions.rs`:
- Around line 561-564: Extend the recursive-call test coverage around the return
handling in the JIT instruction flow to include a function declared with return
type None that executes return recursive_none(n - 1). Verify this path compiles
and returns JitValue::None, preserving the existing Bool and Float recursive
cases.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 6f87064a-97fe-4b9f-8cd2-e433f39bd40b

📥 Commits

Reviewing files that changed from the base of the PR and between 81df1ff and 925912c.

📒 Files selected for processing (3)
  • crates/jit/src/instructions.rs
  • crates/jit/tests/bool_tests.rs
  • crates/jit/tests/float_tests.rs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant