Make use of `Unary` opcodes by ShaharNaveh · Pull Request #6647 · RustPython/RustPython

coderabbitai

📝 Walkthrough

Walkthrough

Replaces the previous generic unary dispatch with explicit per-variant bytecode and intrinsic instructions across codegen, bytecode, VM, and JIT: UnaryPositive, UnaryNegative, UnaryInvert, and UnaryNot. Not now emits ToBool then UnaryNot.

Changes

Cohort / File(s)	Summary
Code generation for expressions `crates/codegen/src/compile.rs`	Emit explicit ops instead of a generic unary: `UAdd` → `CallIntrinsic1(UnaryPositive)`, `USub` → `UnaryNegative`, `Invert` → `UnaryInvert`, `Not` → `ToBool` + `UnaryNot`.
Bytecode instruction definitions `crates/compiler-core/src/bytecode.rs`	Add `UnaryPositive` to `IntrinsicFunction1`; remove `UnaryOperator`/`UnaryOperation`; introduce explicit `UnaryInvert`/`UnaryNegative`/`UnaryNot` instruction variants; update encoding/decoding, stack effects, display, reserved padding, tests/docs.
VM instruction execution `crates/vm/src/frame.rs`	Remove `execute_unary_op` dispatch; add dedicated `UnaryInvert`/`UnaryNegative`/`UnaryNot` arms; route unary plus via `CallIntrinsic1(UnaryPositive)` → `vm._pos(&arg)`; adjust push/pop and intrinsic call paths.
JIT instruction handling `crates/jit/src/instructions.rs`	Replace `UnaryOperation` handling with `ToBool`, `UnaryNot`, and `UnaryNegative` branches; implement boolean-negation and int-negation/invert paths; support `CallIntrinsic1(UnaryPositive)`; update imports to use `IntrinsicFunction1`.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant CG as Codegen
    participant BC as Bytecode
    participant VM as VM
    participant JIT as JIT

    rect rgb(235,245,255)
    Note over CG,BC: Codegen emits explicit unary opcodes/intrinsics
    end

    CG->>BC: Emit CallIntrinsic1(UnaryPositive) / UnaryNegative / UnaryInvert / ToBool + UnaryNot
    BC->>VM: Bytecode stream (explicit unary opcodes)
    VM->>JIT: Request compiled handler or fallback to interpreter

    alt UnaryPositive (UAdd)
        VM->>VM: CallIntrinsic1 handler → vm._pos(arg)
        VM->>VM: Push result
    else UnaryNegative
        VM->>VM: Pop operand → apply negation → Push result
    else UnaryInvert
        VM->>VM: Pop operand → apply bitwise invert (int) or NotSupported → Push result
    else Not (ToBool + UnaryNot)
        VM->>VM: Execute ToBool → UnaryNot (bool flip) → Push result
    end

    rect rgb(245,245,245)
    Note over VM,JIT: Generic UnaryOperation dispatch removed
    end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Sort Instruction enum & related match arms #6322 — Reworks Instruction enum to remove UnaryOperation and add explicit unary variants (strongly related).
New Instruction ToBool,PopJumpIfFalse #6112 — Introduces/adjusts ToBool handling used when lowering Not (related).

Suggested reviewers

youknowone

Poem

🐰 I hopped through opcodes, swift and spry,

Split the unaries, one by one they fly,

calls pos, - and ~ find their lane,
not first converts then flips again,
🥕 tiny hops, the bytecode sings.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Make use of `Unary` opcodes' directly and accurately reflects the main change: replacing a generic unary operation dispatch with explicit unary opcodes (UnaryPositive, UnaryNegative, UnaryNot, UnaryInvert) across the codebase.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 50763ce and 0696124.

📒 Files selected for processing (1)

crates/jit/src/instructions.rs

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

crates/jit/src/instructions.rs

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)

GitHub Check: Run snippets and cpython tests (windows-latest)
GitHub Check: Ensure compilation on various targets
GitHub Check: Run rust tests (windows-latest)
GitHub Check: Run snippets and cpython tests (macos-latest)
GitHub Check: Run rust tests (macos-latest)
GitHub Check: Run rust tests (ubuntu-latest)
GitHub Check: Run snippets and cpython tests on wasm-wasi
GitHub Check: Check the WASM package and demo
GitHub Check: Check Rust code with clippy
GitHub Check: Run snippets and cpython tests (ubuntu-latest)
GitHub Check: Run tests under miri

🔇 Additional comments (3)

crates/jit/src/instructions.rs (3)

6-9: LGTM!

The import of IntrinsicFunction1 is correctly added to support the new CallIntrinsic1 instruction handling.

638-643: LGTM!

The ToBool instruction correctly delegates to boolean_val, which already handles Int, Float, Bool, and None types appropriately.

644-652: Fix confirmed: Type mismatch resolved.

The previous critical issue has been addressed. The code now correctly extracts the Cranelift Value from JitValue::Bool(val) before passing it to bxor_imm. The use of BadBytecode for non-Bool types is appropriate since ToBool is expected to precede UnaryNot in the bytecode sequence.

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 and usage tips.}