Skip to content

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

Merged
youknowone merged 1 commit into
RustPython:mainfrom
kyokuping:codex/jit-recursive-return-type
Aug 12, 2026
Merged

jit: fix recursive call return value type tagging#8499
youknowone merged 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
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: fixing return-value type tagging for recursive JIT calls.
✨ 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

@kyokuping
kyokuping marked this pull request as ready for review August 12, 2026 05:39
@youknowone youknowone added the z-ca-2026 Tag to track Contribution Academy 2026 label Aug 12, 2026

@youknowone youknowone left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tbh, i don't understand this

@youknowone
youknowone merged commit 9de06cc into RustPython:main Aug 12, 2026
27 checks passed
@kyokuping
kyokuping deleted the codex/jit-recursive-return-type branch August 13, 2026 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

z-ca-2026 Tag to track Contribution Academy 2026

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants