Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .cspell.dict/cpython.txt
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ nvars
opname
opnames
orelse
osmodule
outparam
outparm
paramfunc
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ jobs:
with:
openssl: true

# Keep features in sync with update-caches.yml CARGO_ARGS.
- name: build rustpython
run: cargo build --release --verbose --features=threading,jit ${{ env.CARGO_ARGS }}

Expand Down
1 change: 1 addition & 0 deletions .github/workflows/update-caches.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ env:
CARGO_PROFILE_TEST_DEBUG: 0
CARGO_PROFILE_DEV_DEBUG: 0
CARGO_PROFILE_RELEASE_DEBUG: 0
# Keep feature list in sync with CI's release build in .github/workflows/ci.yaml.
CARGO_ARGS: --workspace --no-default-features --features stdlib,importlib,stdio,encodings,sqlite,ssl-rustls-aws-lc,host_env,threading,jit --exclude rustpython_wasm --exclude rustpython-compiler-source --exclude rustpython-venvlauncher

jobs:
Expand Down
2 changes: 1 addition & 1 deletion crates/capi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ rustpython-stdlib = {workspace = true, features = ["threading"] }
rustpython-pylib = { workspace = true }

[dev-dependencies]
pyo3 = { workspace = true, features = ["auto-initialize", "abi3"] }
pyo3 = { workspace = true, features = ["auto-initialize", "abi3t"] }

[lints]
workspace = true
6 changes: 3 additions & 3 deletions crates/capi/pyo3-rustpython.config
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
implementation=CPython
version=3.14
implementation=RustPython
version=3.15

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.

abi3t is only supported from 3.15 onwards. This does not require actual 3.15 runtime support.

shared=true
abi3=true
target_abi=RustPython-abi3t-3.15
suppress_build_script_link_lines=true
37 changes: 37 additions & 0 deletions crates/capi/src/abstract_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,3 +178,40 @@ pub unsafe extern "C" fn PyObject_Size(obj: *mut PyObject) -> isize {
obj.length(vm)
})
}

#[cfg(test)]
mod tests {
use pyo3::prelude::*;
use pyo3::types::{PyDict, PyString};

#[test]
fn call_method1() {
Python::attach(|py| {
let string = PyString::new(py, "Hello, World!");
assert!(
string
.call_method1("endswith", ("!",))
.unwrap()
.is_truthy()
.unwrap()
);
})
}

#[test]
fn object_set_get_del_item() {
Python::attach(|py| {
let obj = PyDict::new(py).into_any();
obj.set_item("key", "value").unwrap();
assert_eq!(
obj.get_item("key")
.unwrap()
.cast_into::<PyString>()
.unwrap(),
"value"
);
obj.del_item("key").unwrap();
assert!(obj.get_item("key").is_err());
})
}
}
2 changes: 1 addition & 1 deletion crates/capi/src/abstract_/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub unsafe extern "C" fn PyIter_Send(
})
}

#[cfg(false)]
#[cfg(test)]
mod tests {
use pyo3::prelude::*;
use pyo3::types::{PyAnyMethods, PyIterator, PyList, PySendResult};
Expand Down
2 changes: 1 addition & 1 deletion crates/capi/src/abstract_/mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ pub unsafe extern "C" fn PyMapping_SetItemString(
})
}

#[cfg(false)]
#[cfg(test)]
mod tests {
use pyo3::prelude::*;
use pyo3::types::{PyDict, PyMapping, PyMappingMethods, PyTuple};
Expand Down
2 changes: 1 addition & 1 deletion crates/capi/src/abstract_/number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ pub unsafe extern "C" fn PyNumber_Subtract(o1: *mut PyObject, o2: *mut PyObject)
with_vm(|vm| vm._sub(unsafe { &*o1 }, unsafe { &*o2 }))
}

#[cfg(false)]
#[cfg(test)]
mod tests {
use pyo3::prelude::*;

Expand Down
2 changes: 1 addition & 1 deletion crates/capi/src/abstract_/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ pub unsafe extern "C" fn PySequence_In(obj: *mut PyObject, value: *mut PyObject)
unsafe { PySequence_Contains(obj, value) }
}

#[cfg(false)]
#[cfg(test)]
mod tests {
use pyo3::prelude::*;
use pyo3::types::{PyAnyMethods, PyDict, PyList, PySequence, PySequenceMethods, PyTuple};
Expand Down
2 changes: 1 addition & 1 deletion crates/capi/src/bytearrayobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub unsafe extern "C" fn PyByteArray_Resize(bytearray: *mut PyObject, len: isize
})
}

#[cfg(false)]
#[cfg(test)]
mod tests {
use pyo3::prelude::*;
use pyo3::types::{PyByteArray, PyBytes};
Expand Down
9 changes: 4 additions & 5 deletions crates/capi/src/bytesobject.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use crate::PyObject;
use crate::object::define_py_check;
use crate::pystate::with_vm;
use crate::{PyObject, pystate::with_vm};
use core::ffi::c_char;
use rustpython_vm::builtins::PyBytes;

Expand Down Expand Up @@ -46,21 +45,21 @@ pub unsafe extern "C" fn PyBytes_AsString(bytes: *mut PyObject) -> *mut c_char {
})
}

#[cfg(false)]
#[cfg(test)]
mod tests {
use pyo3::prelude::*;
use pyo3::types::PyBytes;

#[test]
fn test_bytes() {
fn bytes() {
Python::attach(|py| {
let bytes = PyBytes::new(py, b"Hello, World!");
assert_eq!(bytes.as_bytes(), b"Hello, World!");
})
}

#[test]
fn test_bytes_uninit() {
fn bytes_uninit() {
Python::attach(|py| {
let bytes = PyBytes::new_with(py, 13, |data| {
data.copy_from_slice(b"Hello, World!");
Expand Down
6 changes: 3 additions & 3 deletions crates/capi/src/ceval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,21 @@ pub extern "C" fn PyEval_GetBuiltins() -> *mut PyObject {
})
}

#[cfg(false)]
#[cfg(test)]
mod tests {
use pyo3::exceptions::PyException;
use pyo3::prelude::*;

#[test]
fn test_code_eval() {
fn code_eval() {
Python::attach(|py| {
let result = py.eval(c"1 + 1", None, None).unwrap();
assert_eq!(result.extract::<u32>().unwrap(), 2);
})
}

#[test]
fn test_code_run_exception() {
fn code_run_exception() {
Python::attach(|py| {
let err = py.run(c"raise Exception()", None, None).unwrap_err();
assert!(err.is_instance_of::<PyException>(py));
Expand Down
4 changes: 2 additions & 2 deletions crates/capi/src/complexobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ pub unsafe extern "C" fn PyComplex_ImagAsDouble(obj: *mut PyObject) -> c_double
with_vm(|vm| try_to_complex(vm, unsafe { &*obj }).map(|complex| complex.im))
}

#[cfg(false)]
#[cfg(test)]
mod tests {
use pyo3::prelude::*;
use pyo3::types::PyComplex;

#[test]
fn test_py_int() {
fn py_int() {
Python::attach(|py| {
let number = PyComplex::from_doubles(py, 1.0, 2.0);
assert_eq!(number.real(), 1.0);
Expand Down
31 changes: 31 additions & 0 deletions crates/capi/src/critical_section.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
use crate::PyObject;

#[repr(C)]
pub struct PyCriticalSection;

#[repr(C)]
pub struct PyCriticalSection2;

#[unsafe(no_mangle)]
pub extern "C" fn PyCriticalSection_Begin(c: *mut PyCriticalSection, op: *mut PyObject) {
let _ = (c, op);
}

#[unsafe(no_mangle)]
pub extern "C" fn PyCriticalSection_End(c: *mut PyCriticalSection) {
let _ = c;
}

#[unsafe(no_mangle)]
pub extern "C" fn PyCriticalSection2_Begin(
c: *mut PyCriticalSection2,
a: *mut PyObject,
b: *mut PyObject,
) {
let _ = (c, a, b);
}

#[unsafe(no_mangle)]
pub extern "C" fn PyCriticalSection2_End(c: *mut PyCriticalSection2) {
let _ = c;
}
4 changes: 2 additions & 2 deletions crates/capi/src/descrobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub unsafe extern "C" fn PyDictProxy_New(mapping: *mut PyObject) -> *mut PyObjec
})
}

#[cfg(false)]
#[cfg(test)]
mod tests {
use pyo3::prelude::*;
use pyo3::types::{PyDict, PyInt, PyMappingProxy};
Expand All @@ -23,7 +23,7 @@ mod tests {
dict.set_item("x", 7).unwrap();

let mapping = dict.as_mapping();
let proxy = PyMappingProxy::new(py, &mapping);
let proxy = PyMappingProxy::new(py, mapping);
let value = proxy.get_item("x").unwrap().cast_into::<PyInt>().unwrap();
assert_eq!(value, 7);
})
Expand Down
45 changes: 41 additions & 4 deletions crates/capi/src/dictobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,43 @@ pub unsafe extern "C" fn PyDict_GetItemRef(
})
}

#[unsafe(no_mangle)]
pub unsafe extern "C" fn PyDict_SetDefaultRef(
dict: *mut PyObject,
key: *mut PyObject,
default_value: *mut PyObject,
result: *mut *mut PyObject,
) -> c_int {
with_vm(|vm| {
let result = NonNull::new(result);
if let Some(result) = result {
unsafe {
result.write(core::ptr::null_mut());
}
}
let dict = unsafe { &*dict }.try_downcast_ref::<PyDict>(vm)?;
let key = unsafe { &*key };

if let Some(value) = dict.inner_getitem_opt(key, vm)? {
if let Some(result) = result {
unsafe {
result.write(value.into_raw().as_ptr());
}
}
Ok(true)
} else {
let value = unsafe { &*default_value }.to_owned();
dict.inner_setitem(key, value.clone(), vm)?;
Comment on lines +77 to +86

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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd crates/capi/src && wc -l dictobject.rs

Repository: RustPython/RustPython

Length of output: 83


🏁 Script executed:

cd crates/capi/src && sed -n '60,100p' dictobject.rs

Repository: RustPython/RustPython

Length of output: 1332


🏁 Script executed:

# Search for setdefault or entry patterns in dict implementation
fd -e rs | xargs rg -l "setdefault|entry" | head -5

Repository: RustPython/RustPython

Length of output: 182


🏁 Script executed:

# Look for the dict module and check for setdefault implementations
fd -e rs | xargs rg -A 5 "fn setdefault" | head -50

Repository: RustPython/RustPython

Length of output: 1597


🏁 Script executed:

# Get full context of setdefault_entry
fd -e rs dict_inner.rs | xargs sed -n '1,500p' | grep -A 30 "pub(crate) fn setdefault_entry"

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Search in dict_inner.rs for the complete setdefault_entry and setdefault implementations
rg -A 20 "pub\(crate\) fn setdefault" crates/vm/src/dict_inner.rs

Repository: RustPython/RustPython

Length of output: 1599


🏁 Script executed:

# Check the PyDict struct and see what methods are available
rg -B 5 -A 15 "impl PyDict" crates/pyobject/src/py_dict.rs | head -100

Repository: RustPython/RustPython

Length of output: 136


🏁 Script executed:

# Look for inner_getitem_opt and inner_setitem in PyDict to understand the current approach
rg "inner_getitem_opt|inner_setitem" crates/pyobject/src/py_dict.rs | head -20

Repository: RustPython/RustPython

Length of output: 136


🏁 Script executed:

# Find where PyDict is defined
fd -e rs | xargs rg "struct PyDict" | head -10

Repository: RustPython/RustPython

Length of output: 116


🏁 Script executed:

# Look at the full setdefault_entry implementation to see what it returns
rg -A 30 "pub\(crate\) fn setdefault_entry" crates/vm/src/dict_inner.rs

Repository: RustPython/RustPython

Length of output: 1245


🏁 Script executed:

# Check what dict module/PyDict implementations look like
fd -e rs | xargs rg "impl.*PyDict" | head -20

Repository: RustPython/RustPython

Length of output: 1244


🏁 Script executed:

# Find where inner_getitem_opt and inner_setitem are defined for PyDict
rg "fn inner_getitem_opt|fn inner_setitem" crates --type rs | head -20

Repository: RustPython/RustPython

Length of output: 92


🏁 Script executed:

# Search in builtins/dict.rs for inner_getitem_opt and inner_setitem
rg "fn inner_getitem_opt|fn inner_setitem" crates/vm/src/builtins/dict.rs

Repository: RustPython/RustPython

Length of output: 163


🏁 Script executed:

# Get the full view of how PyDict methods work
sed -n '1,100p' crates/vm/src/builtins/dict.rs

Repository: RustPython/RustPython

Length of output: 3266


🏁 Script executed:

# Check what the setdefault implementation in PyDict looks like
rg -B 5 -A 20 "fn setdefault" crates/vm/src/builtins/dict.rs

Repository: RustPython/RustPython

Length of output: 609


🏁 Script executed:

# Look for how inner_getitem_opt delegates and what it calls
rg -A 10 "inner_getitem_opt" crates/vm/src/builtins/dict.rs | head -50

Repository: RustPython/RustPython

Length of output: 296


🏁 Script executed:

# See what inner_setitem actually does
rg -A 15 "pub fn inner_setitem" crates/vm/src/builtins/dict.rs

Repository: RustPython/RustPython

Length of output: 458


🏁 Script executed:

# Verify that inner_getitem_opt and inner_setitem each trigger separate dict operations
rg -B 2 -A 8 "pub fn inner_getitem_opt" crates/vm/src/builtins/dict.rs

Repository: RustPython/RustPython

Length of output: 268


🏁 Script executed:

# Check the full setdefault_entry to see if we can adapt it for the C API use case
rg -A 40 "pub\(crate\) fn setdefault_entry" crates/vm/src/dict_inner.rs | head -60

Repository: RustPython/RustPython

Length of output: 1439


Use dict_inner::Dict::setdefault_entry to avoid double-hashing the key.

The current implementation calls inner_getitem_opt() and then inner_setitem(), which performs two separate dictionary operations. dict_inner::Dict::setdefault_entry() exists and performs a single hash evaluation (let hash = key.key_hash(vm)?;) for both lookup and insertion. This matters for keys with observable __hash__/__eq__ side effects and preserves the atomicity contract of the C API. Route through dict.entries.setdefault_entry() with a closure that wraps the default value, which returns (PyObjectRef, PyObjectRef) containing both the final value and the key for error handling.

🤖 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/dictobject.rs` around lines 77 - 86, The current
implementation performs double-hashing by calling inner_getitem_opt() and then
inner_setitem() separately on the dict object. Replace this approach with
dict.entries.setdefault_entry() which performs a single hash evaluation for both
lookup and insertion operations. Pass a closure to setdefault_entry() that wraps
the default_value, and handle the returned tuple containing both the final value
and the key. This eliminates the double-hashing and preserves the atomicity
contract of the C API, which is important for keys with observable side effects
in __hash__ or __eq__.

if let Some(result) = result {
unsafe {
result.write(value.into_raw().as_ptr());
}
}
Ok(false)
}
})
}

#[unsafe(no_mangle)]
pub unsafe extern "C" fn PyDict_Size(dict: *mut PyObject) -> isize {
with_vm(|vm| {
Expand Down Expand Up @@ -187,21 +224,21 @@ pub unsafe extern "C" fn PyDict_Next(
})
}

#[cfg(false)]
#[cfg(test)]
mod tests {
use pyo3::prelude::*;
use pyo3::types::{IntoPyDict, PyDict, PyDictMethods, PyInt, PyList};

#[test]
fn test_create_empty_dict() {
fn create_empty_dict() {
Python::attach(|py| {
let dict = PyDict::new(py);
assert!(dict.is_instance_of::<PyDict>());
})
}

#[test]
fn test_create_dict_with_items() {
fn create_dict_with_items() {
Python::attach(|py| {
let dict = [(1, 2), (3, 4)].into_py_dict(py)?;
let value = dict.get_item(1)?.unwrap().cast_into::<PyInt>()?;
Expand All @@ -214,7 +251,7 @@ mod tests {
}

#[test]
fn test_dict_iter() {
fn dict_iter() {
Python::attach(|py| {
let dict = [(1, 2), (3, 4)].into_py_dict(py).unwrap();
let values = dict
Expand Down
4 changes: 2 additions & 2 deletions crates/capi/src/floatobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ pub unsafe extern "C" fn PyFloat_FromString(obj: *mut PyObject) -> *mut PyObject
})
}

#[cfg(false)]
#[cfg(test)]
mod tests {
use core::f64::consts::PI;
use pyo3::prelude::*;
use pyo3::types::PyFloat;

#[test]
fn test_py_float() {
fn py_float() {
Python::attach(|py| {
let pi = PyFloat::new(py, PI);
assert!(pi.is_instance_of::<PyFloat>());
Expand Down
15 changes: 15 additions & 0 deletions crates/capi/src/genericaliasobject.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
use crate::{PyObject, pystate::with_vm};
use rustpython_vm::PyPayload;
use rustpython_vm::builtins::PyGenericAlias;

#[unsafe(no_mangle)]
pub unsafe extern "C" fn Py_GenericAlias(
origin: *mut PyObject,
args: *mut PyObject,
) -> *mut PyObject {
with_vm(|vm| {
let origin = unsafe { &*origin }.to_owned();
let args = unsafe { &*args }.to_owned();
PyGenericAlias::from_args(origin, args, vm).into_pyobject(vm)
})
}
Loading
Loading