Skip to content

Add eval function to c-api#7927

Merged
youknowone merged 3 commits into
RustPython:mainfrom
bschoenmaeckers:c-api-eval
May 20, 2026
Merged

Add eval function to c-api#7927
youknowone merged 3 commits into
RustPython:mainfrom
bschoenmaeckers:c-api-eval

Conversation

@bschoenmaeckers
Copy link
Copy Markdown
Contributor

@bschoenmaeckers bschoenmaeckers commented May 19, 2026

Summary by CodeRabbit

  • New Features
    • Added C API entry points to compile and evaluate Python code and to access VM builtins, enabling embedding and executing Python with custom globals/locals.
  • Chores
    • Updated a crate dependency to enable compiler-related features required by the new C API.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 28f01d3c-8192-4af7-8c20-d66cf9c47e28

📥 Commits

Reviewing files that changed from the base of the PR and between 9057729 and 202de7a.

📒 Files selected for processing (1)
  • crates/capi/src/ceval.rs

📝 Walkthrough

Walkthrough

Adds a new public ceval module and three C-API FFI functions: Py_CompileString, PyEval_EvalCode, and PyEval_GetBuiltins. Enables the compiler feature on the rustpython-vm dependency.

Changes

Python C-API Code Evaluation and Compilation

Layer / File(s) Summary
Compiler feature enablement
crates/capi/Cargo.toml
RustPython VM compiler feature added to the dependency features alongside threading.
Module declaration
crates/capi/src/lib.rs
The ceval module is publicly exported from the capi crate.
FFI imports and compile-mode constants
crates/capi/src/ceval.rs
Adds imports and local PY_* compile-mode constants used by the FFI compile entry point.
FFI: Py_CompileString
crates/capi/src/ceval.rs
Implements Py_CompileString: validates C strings, maps start to compiler Mode, calls the VM compiler, and converts compile errors to Python syntax errors or system errors.
FFI: PyEval_EvalCode
crates/capi/src/ceval.rs
Implements PyEval_EvalCode: downcasts co/globals, optionally converts locals to ArgMapping, constructs Scope with builtins, and runs the code object via the VM.
PyEval_GetBuiltins and tests
crates/capi/src/ceval.rs
Implements PyEval_GetBuiltins returning current-frame or VM builtins pointer; adds a #[cfg(false)] test module (disabled).

Sequence Diagram(s)

sequenceDiagram
  participant C as C_client
  participant CevalCompile as capi::ceval::Py_CompileString
  participant VM as rustpython_vm::VM
  C->>CevalCompile: pass `code`, `filename`, `start` (C strings)
  CevalCompile->>VM: convert strings, call compile(mode)
  VM-->>CevalCompile: Compiled PyCode or CompileError
  CevalCompile-->>C: return PyObject* or raise SyntaxError/SystemError
Loading
sequenceDiagram
  participant C as C_client
  participant CevalEval as capi::ceval::PyEval_EvalCode
  participant VM as rustpython_vm::VM
  participant Builtins as VM::builtins
  C->>CevalEval: pass `co`, `globals`, `locals`
  CevalEval->>VM: prepare Scope(globals, locals, Builtins)
  CevalEval->>VM: VM.run_code_object(PyCode, Scope)
  VM-->>CevalEval: result PyObject or exception
  CevalEval-->>C: return PyObject* or exception
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • youknowone
  • ShaharNaveh

Poem

A rabbit taps keys in moonlit labs,
Compiling strings across FFI slabs.
Builtins hop in, scope stitched tight,
Rust and Python dance through night. 🐇✨

🚥 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 'Add eval function to c-api' clearly and accurately describes the main change, which adds three FFI entry points (Py_CompileString, PyEval_EvalCode, PyEval_GetBuiltins) related to evaluation and compilation functionality to the RustPython C API.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
crates/capi/src/ceval.rs (1)

64-84: 🏗️ Heavy lift

This test module doesn't cover the new FFI surface.

#[cfg(false)] keeps it permanently disabled, and the assertions call PyO3's eval/run APIs rather than Py_CompileString or PyEval_EvalCode. Please replace this with real coverage for the exported C-API entrypoints or drop the dead module.

🤖 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/capi/src/ceval.rs` around lines 64 - 84, The disabled test module (mod
tests) uses #[cfg(false)] and exercises PyO3's Python::eval/run rather than the
crate's exported C-API; either remove this dead module or replace it with tests
that call the real FFI surface (e.g., tests that invoke the exported functions
which wrap Py_CompileString and PyEval_EvalCode) — update or add test functions
(replace test_code_eval and test_code_run_exception) to call the crate's C-API
entrypoints, assert correct return values and that exceptions from the FFI path
produce PyException instances, and remove #[cfg(false)] so the tests run.
🤖 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/capi/src/ceval.rs`:
- Around line 59-61: PyEval_GetBuiltins currently returns the interpreter-wide
vm.builtins unconditionally; change it to check vm.current_frame() and, if
Some(frame), return that frame's InterpreterFrame.builtins raw pointer,
otherwise fall back to vm.builtins. Update the implementation inside with_vm to
call vm.current_frame(), access frame.builtins (via as_object().as_raw() or
equivalent), and only use vm.builtins when no current frame is present so the
function honors per-frame __builtins__ overrides.
- Around line 11-22: The FFI functions (notably Py_CompileString and other C
entrypoints that call CStr::from_ptr or dereference raw pointers like &*co and
&*globals) must validate incoming pointers before dereferencing to avoid UB:
check that raw pointers (code, filename, co, globals) are non-null (use
NonNull::new or explicit null checks) and return an appropriate Python exception
via the VM when null or invalid UTF-8 occurs; then safely call CStr::from_ptr
and only dereference pointers after wrapping them in NonNull and converting to
references. Ensure you mirror the existing locals pattern (NonNull::new(locals))
for code, filename, co, globals and produce the VM error return path instead of
dereferencing null.

---

Nitpick comments:
In `@crates/capi/src/ceval.rs`:
- Around line 64-84: The disabled test module (mod tests) uses #[cfg(false)] and
exercises PyO3's Python::eval/run rather than the crate's exported C-API; either
remove this dead module or replace it with tests that call the real FFI surface
(e.g., tests that invoke the exported functions which wrap Py_CompileString and
PyEval_EvalCode) — update or add test functions (replace test_code_eval and
test_code_run_exception) to call the crate's C-API entrypoints, assert correct
return values and that exceptions from the FFI path produce PyException
instances, and remove #[cfg(false)] so the tests run.
🪄 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: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 34637ca5-adfb-4270-8cf4-97d2937d543d

📥 Commits

Reviewing files that changed from the base of the PR and between 26f5bbf and 37efb05.

📒 Files selected for processing (3)
  • crates/capi/Cargo.toml
  • crates/capi/src/ceval.rs
  • crates/capi/src/lib.rs

Comment thread crates/capi/src/ceval.rs
Comment thread crates/capi/src/ceval.rs Outdated
Comment thread crates/capi/src/ceval.rs Outdated
.map_err(|_| vm.new_system_error("Py_CompileString called with non UTF-8 filename"))?;

let mode = match start {
256 => Mode::Single,
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.

what's these numbers? do they have proper name or symbol?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've added constants for these values.

Copy link
Copy Markdown
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

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

👍

@youknowone youknowone merged commit de06fc0 into RustPython:main May 20, 2026
26 checks passed
@bschoenmaeckers bschoenmaeckers deleted the c-api-eval branch May 20, 2026 13:20
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.

2 participants