Skip to content

Commit

Permalink
Remove get_refcnt from public api
Browse files Browse the repository at this point in the history
This is an attempt to fix PyO3#3357.
The function is used relatively frequently in tests,
so have left it with #[cfg(test)] and pub(crate).
This avoids coating the tests with `unsafe`,
and something that doesn't indicate intent as well.

TODO:
[ ] Fix docs that refer to `get_refcnt` as an example -
    is there another function I can use instead?
  • Loading branch information
lfn3 committed Apr 10, 2024
1 parent 2f0869a commit c9857d4
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 15 deletions.
1 change: 0 additions & 1 deletion src/gil.rs
Expand Up @@ -521,7 +521,6 @@ mod tests {
#[allow(deprecated)]
use super::GILPool;
use super::{gil_is_acquired, GIL_COUNT, OWNED_OBJECTS, POOL};
use crate::types::any::PyAnyMethods;
use crate::{ffi, gil, PyObject, Python};
#[cfg(not(target_arch = "wasm32"))]
use parking_lot::{const_mutex, Condvar, Mutex};
Expand Down
11 changes: 9 additions & 2 deletions src/instance.rs
Expand Up @@ -202,6 +202,14 @@ impl<'py> Bound<'py, PyAny> {
) -> &'a Option<Self> {
&*(ptr as *const *mut ffi::PyObject).cast::<Option<Bound<'py, PyAny>>>()
}

/// Returns the reference count for the Python object.
///
/// Only used to validate behaviour in tests.
#[cfg(test)]
pub(crate) fn get_refcnt(&self) -> isize {
unsafe { ffi::Py_REFCNT(self.as_ptr()) }
}
}

impl<'py, T> Bound<'py, T>
Expand Down Expand Up @@ -1288,8 +1296,7 @@ impl<T> Py<T> {
}

/// Gets the reference count of the `ffi::PyObject` pointer.
#[inline]
pub fn get_refcnt(&self, _py: Python<'_>) -> isize {
pub(crate) fn get_refcnt(&self, _py: Python<'_>) -> isize {
unsafe { ffi::Py_REFCNT(self.0.as_ptr()) }
}

Expand Down
12 changes: 4 additions & 8 deletions src/types/any.rs
Expand Up @@ -810,7 +810,10 @@ impl PyAny {
}

/// Returns the reference count for the Python object.
pub fn get_refcnt(&self) -> isize {
///
/// Only used to validate behaviour in tests.
#[cfg(test)]
pub(crate) fn get_refcnt(&self) -> isize {
self.as_borrowed().get_refcnt()
}

Expand Down Expand Up @@ -1648,9 +1651,6 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed {
where
T: FromPyObjectBound<'a, 'py>;

/// Returns the reference count for the Python object.
fn get_refcnt(&self) -> isize;

/// Computes the "repr" representation of self.
///
/// This is equivalent to the Python expression `repr(self)`.
Expand Down Expand Up @@ -2188,10 +2188,6 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> {
FromPyObjectBound::from_py_object_bound(self.as_borrowed())
}

fn get_refcnt(&self) -> isize {
unsafe { ffi::Py_REFCNT(self.as_ptr()) }
}

fn repr(&self) -> PyResult<Bound<'py, PyString>> {
unsafe {
ffi::PyObject_Repr(self.as_ptr())
Expand Down
8 changes: 4 additions & 4 deletions tests/test_inheritance.rs
Expand Up @@ -242,16 +242,16 @@ mod inheriting_native_type {
fn inherit_dict_drop() {
Python::with_gil(|py| {
let dict_sub = pyo3::Py::new(py, DictWithName::new()).unwrap();
assert_eq!(dict_sub.get_refcnt(py), 1);
assert_eq!(unsafe { pyo3::ffi::Py_REFCNT(dict_sub.as_ptr()) }, 1);

let item = &py.eval_bound("object()", None, None).unwrap();
assert_eq!(item.get_refcnt(), 1);
assert_eq!(unsafe { pyo3::ffi::Py_REFCNT(item.as_ptr()) }, 1);

dict_sub.bind(py).set_item("foo", item).unwrap();
assert_eq!(item.get_refcnt(), 2);
assert_eq!(unsafe { pyo3::ffi::Py_REFCNT(item.as_ptr()) }, 2);

drop(dict_sub);
assert_eq!(item.get_refcnt(), 1);
assert_eq!(unsafe { pyo3::ffi::Py_REFCNT(item.as_ptr()) }, 1);
})
}

Expand Down

0 comments on commit c9857d4

Please sign in to comment.