Implement suggestions from review PyO3#1115

src/err/err_state.rs

@@ -1,8 +1,13 @@
 use crate::{
-    err::PyErrValue, exceptions::PyBaseException, ffi, types::PyType, IntoPyPointer, Py, PyObject,
-    Python,
+    exceptions::PyBaseException, ffi, types::PyType, IntoPyPointer, Py, PyObject,
+    Python, ToPyObject,
 };
 
+pub(crate) enum PyErrValue {
+    ToArgs(Box<dyn PyErrArguments + Send + Sync>),
+    ToObject(Box<dyn ToPyObject + Send + Sync>),
+}
+
 #[derive(Clone)]
 pub(crate) struct PyErrStateNormalized {
     pub ptype: Py<PyType>,
@@ -23,6 +28,12 @@ pub(crate) enum PyErrState {
     Normalized(PyErrStateNormalized),
 }
 
+/// Helper conversion trait that allows to use custom arguments for exception constructor.
+pub trait PyErrArguments {
+    /// Arguments for exception
+    fn arguments(&self, _: Python) -> PyObject;
+}
+
 impl PyErrState {
     pub fn into_ffi_tuple(
         self,

src/err/mod.rs

@@ -20,13 +20,10 @@ use std::os::raw::c_char;
 use std::ptr::NonNull;
 
 mod err_state;
-mod err_value;
 mod impls;
 
-use err_state::{PyErrState, PyErrStateNormalized};
-use err_value::PyErrValue;
-
-pub use err_value::PyErrArguments;
+use err_state::{PyErrState, PyErrStateNormalized, PyErrValue};
+pub use err_state::PyErrArguments;
 
 /// Represents a Python exception that was raised.
 pub struct PyErr {
@@ -76,8 +73,7 @@ impl PyErr {
     /// return Err(PyErr::new::<exceptions::PyTypeError, _>("Error message"));
     /// ```
     ///
-    /// In most cases, you can use a concrete exception's constructors instead:
-    /// the example is equivalent to
+    /// In most cases, you can use a concrete exception's constructor instead, which is equivalent:
     /// ```ignore
     /// return Err(exceptions::PyTypeError::new_err("Error message"));
     /// ```
@@ -407,6 +403,12 @@ impl PyErr {
     }
 
     fn normalized(&self, py: Python) -> &PyErrStateNormalized {
+        // This process is safe because:
+        // - Access is guaranteed not to be concurrent thanks to `Python` GIL token
+        // - Write happens only once, and then never will change again.
+        // - State is set to None during the normalization process, so that a second
+        //   concurrent normalization attempt will panic before changing anything.
+
         if let Some(PyErrState::Normalized(n)) = unsafe { &*self.state.get() } {
             return n;
         }