add safety comments (#6139)

Signed-off-by: person93 <person93.person93@gmail.com>

Author: Person-93

src/conversions/std/osstr.rs

@@ -1,6 +1,3 @@
-// TODO https://github.com/PyO3/pyo3/issues/5487
-#![allow(clippy::undocumented_unsafe_blocks)]
-
 use crate::conversion::IntoPyObject;
 #[cfg(not(target_os = "wasi"))]
 use crate::ffi;
@@ -50,6 +47,7 @@ impl<'py> IntoPyObject<'py> for &OsStr {
             let bytes = self.as_bytes();
             let ptr = bytes.as_ptr().cast();
             let len = bytes.len() as ffi::Py_ssize_t;
+            // SAFETY: passing valid pointer to python API
             unsafe {
                 // DecodeFSDefault automatically chooses an appropriate decoding mechanism to
                 // parse os strings losslessly (i.e. surrogateescape most of the time)
@@ -62,6 +60,7 @@ impl<'py> IntoPyObject<'py> for &OsStr {
         #[cfg(windows)]
         {
             let wstr: Vec<u16> = self.encode_wide().collect();
+            // SAFETY: passing valid pointer to python API
             unsafe {
                 // This will not panic because the data from encode_wide is well-formed Windows
                 // string data
@@ -130,6 +129,7 @@ impl FromPyObject<'_, '_> for OsString {
 
             // Get an owned allocated wide char buffer from PyString, which we have to deallocate
             // ourselves
+            // SAFETY: passing valid pointer to python API
             let size =
                 unsafe { ffi::PyUnicode_AsWideChar(pystring.as_ptr(), core::ptr::null_mut(), 0) };
             crate::err::error_on_minusone(ob.py(), size)?;
@@ -141,6 +141,7 @@ impl FromPyObject<'_, '_> for OsString {
             let size = size - 1; // exclude null terminator
 
             let mut buffer = vec![0; size as usize];
+            // SAFETY: passing valid pointer to python API
             let bytes_read =
                 unsafe { ffi::PyUnicode_AsWideChar(pystring.as_ptr(), buffer.as_mut_ptr(), size) };
             assert_eq!(bytes_read, size);

src/exceptions.rs

@@ -1,6 +1,3 @@
-// TODO https://github.com/PyO3/pyo3/issues/5487
-#![allow(clippy::undocumented_unsafe_blocks)]
-
 //! Exception and warning types defined by Python.
 //!
 //! The structs in this module represent Python's built-in exceptions and
@@ -280,7 +277,11 @@ macro_rules! impl_native_exception (
         pub struct $name($crate::PyAny);
 
         $crate::impl_exception_boilerplate!($name);
-        $crate::pyobject_native_type!($name, $layout, |_py| unsafe { $crate::ffi::$exc_name as *mut $crate::ffi::PyTypeObject }, "builtins", $python_name $(, #checkfunction=$checkfunction)?);
+        $crate::pyobject_native_type!($name, $layout, |_py| {
+            // SAFETY: cpython docs state that all exception types are available as global variales and are class objects
+            //         https://docs.python.org/3/c-api/exceptions.html#exception-and-warning-types
+            unsafe { $crate::ffi::$exc_name as *mut $crate::ffi::PyTypeObject }
+        }, "builtins", $python_name $(, #checkfunction=$checkfunction)?);
         $crate::pyobject_subclassable_native_type!($name, $layout);
     );
     ($name:ident, $exc_name:ident, $python_name:expr, $doc:expr) => (
@@ -730,6 +731,7 @@ impl PyUnicodeDecodeError {
     ) -> PyResult<Bound<'py, PyUnicodeDecodeError>> {
         use crate::ffi_ptr_ext::FfiPtrExt;
         use crate::py_result_ext::PyResultExt;
+        // SAFETY: calling python API with correct pointers
         unsafe {
             ffi::PyUnicodeDecodeError_Create(
                 encoding.as_ptr(),

src/ffi_ptr_ext.rs

@@ -1,6 +1,3 @@
-// TODO https://github.com/PyO3/pyo3/issues/5487
-#![allow(clippy::undocumented_unsafe_blocks)]
-
 use crate::sealed::Sealed;
 use crate::{
     ffi,
@@ -41,48 +38,80 @@ pub(crate) trait FfiPtrExt: Sealed {
 }
 
 impl FfiPtrExt for *mut ffi::PyObject {
+    /// # Safety
+    ///
+    /// see requirements for [`Bound::from_owned_ptr_or_err`]
     #[inline]
     unsafe fn assume_owned_or_err(self, py: Python<'_>) -> PyResult<Bound<'_, PyAny>> {
+        // SAFETY: caller upholds requirements
         unsafe { Bound::from_owned_ptr_or_err(py, self) }
     }
 
+    /// # Safety
+    ///
+    /// see requirements for [`Bound::from_owned_ptr_or_opt`]
     #[inline]
     unsafe fn assume_owned_or_opt(self, py: Python<'_>) -> Option<Bound<'_, PyAny>> {
+        // SAFETY: caller upholds requirements
         unsafe { Bound::from_owned_ptr_or_opt(py, self) }
     }
 
+    /// # Safety
+    ///
+    /// see requirements for [`Bound::from_owned_ptr`]
     #[inline]
     #[track_caller]
     unsafe fn assume_owned(self, py: Python<'_>) -> Bound<'_, PyAny> {
+        // SAFETY: caller upholds requirements
         unsafe { Bound::from_owned_ptr(py, self) }
     }
 
+    /// # Safety
+    ///
+    /// see requirements for [`Bound::from_owned_ptr_unchecked`]
     #[inline]
     unsafe fn assume_owned_unchecked(self, py: Python<'_>) -> Bound<'_, PyAny> {
+        // SAFETY: caller upholds requirements
         unsafe { Bound::from_owned_ptr_unchecked(py, self) }
     }
 
+    /// # Safety
+    ///
+    /// see requirements for [`Borrowed::from_ptr_or_err`]
     #[inline]
     unsafe fn assume_borrowed_or_err<'a>(
         self,
         py: Python<'_>,
     ) -> PyResult<Borrowed<'a, '_, PyAny>> {
+        // SAFETY: caller upholds requirements
         unsafe { Borrowed::from_ptr_or_err(py, self) }
     }
 
+    /// # Safety
+    ///
+    /// see requirements for [`Borrowed::from_ptr_or_opt`]
     #[inline]
     unsafe fn assume_borrowed_or_opt<'a>(self, py: Python<'_>) -> Option<Borrowed<'a, '_, PyAny>> {
+        // SAFETY: caller upholds requirements
         unsafe { Borrowed::from_ptr_or_opt(py, self) }
     }
 
+    /// # Safety
+    ///
+    /// see requirements for [`Borrowed::from_ptr`]
     #[inline]
     #[track_caller]
     unsafe fn assume_borrowed<'a>(self, py: Python<'_>) -> Borrowed<'a, '_, PyAny> {
+        // SAFETY: caller upholds requirements
         unsafe { Borrowed::from_ptr(py, self) }
     }
 
+    /// # Safety
+    ///
+    /// see requirements for [`Borrowed::from_ptr_unchecked`]
     #[inline]
     unsafe fn assume_borrowed_unchecked<'a>(self, py: Python<'_>) -> Borrowed<'a, '_, PyAny> {
+        // SAFETY: caller upholds requirements
         unsafe { Borrowed::from_ptr_unchecked(py, self) }
     }
 }

src/sync.rs

@@ -88,11 +88,13 @@ impl<T> Default for GILOnceCell<T> {
     }
 }
 
+// SAFETY: Sync is only implemented if the inner type is Sync
 // T: Send is needed for Sync because the thread which drops the GILOnceCell can be different
 // to the thread which fills it. (e.g. think scoped thread which fills the cell and then exits,
 // leaving the cell to be dropped by the main thread).
 #[allow(deprecated)]
 unsafe impl<T: Send + Sync> Sync for GILOnceCell<T> {}
+// SAFETY: send is only implemented if the inner type is send
 #[allow(deprecated)]
 unsafe impl<T: Send> Send for GILOnceCell<T> {}
 

src/sync/critical_section.rs

@@ -1,3 +1,6 @@
+// TODO https://github.com/PyO3/pyo3/issues/5487
+#![allow(clippy::undocumented_unsafe_blocks)]
+
 //! Wrappers for the Python critical section API
 //!
 //! [Critical Sections](https://docs.python.org/3/c-api/init.html#python-critical-section-api) allow

src/types/mod.rs

@@ -241,6 +241,7 @@ macro_rules! pyobject_subclassable_native_type {
 #[macro_export]
 macro_rules! pyobject_native_type_sized {
     ($name:ty, $layout:path $(;$generics:ident)*) => {
+        // SAFETY: native objects are valid
         unsafe impl $crate::type_object::PyLayout<$name> for $layout {}
         impl $crate::type_object::PySizedLayout<$name> for $layout {}
     };