From 355bd0c336ab7cadbc6ea10e40d77b6eb40b8ebd Mon Sep 17 00:00:00 2001
From: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
Date: Tue, 25 May 2021 08:02:12 +0100
Subject: [PATCH] pymodule: tidy up module init

---
 CHANGELOG.md                      |  1 +
 pyo3-macros-backend/src/module.rs |  7 ++---
 src/derive_utils.rs               | 43 +++++++++++++++++--------------
 src/ffi/moduleobject.rs           | 16 ------------
 4 files changed, 28 insertions(+), 39 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ee4e399e..581d5906 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -57,6 +57,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Remove pyclass implementation details from `PyTypeInfo`:
   - `Type`, `DESCRIPTION`, and `FLAGS` [#1456](https://github.com/PyO3/pyo3/pull/1456)
   - `BaseType`, `BaseLayout`, `Layout`, `Initializer` [#1596](https://github.com/PyO3/pyo3/pull/1596)
+  - `PyModuleDef_INIT` [#1630](https://github.com/PyO3/pyo3/pull/1630)
 - Remove `__doc__` from module's `__all__`. [#1509](https://github.com/PyO3/pyo3/pull/1509)
 - Remove `PYO3_CROSS_INCLUDE_DIR` environment variable and the associated C header parsing functionality. [#1521](https://github.com/PyO3/pyo3/pull/1521)
 
diff --git a/pyo3-macros-backend/src/module.rs b/pyo3-macros-backend/src/module.rs
index 5bc8962e..911436a3 100644
--- a/pyo3-macros-backend/src/module.rs
+++ b/pyo3-macros-backend/src/module.rs
@@ -22,10 +22,11 @@ pub fn py_init(fnname: &Ident, name: &Ident, doc: syn::LitStr) -> TokenStream {
         /// the module.
         pub unsafe extern "C" fn #cb_name() -> *mut pyo3::ffi::PyObject {
             use pyo3::derive_utils::ModuleDef;
-            const NAME: &'static str = concat!(stringify!(#name), "\0");
-            static MODULE_DEF: ModuleDef = unsafe { ModuleDef::new(NAME) };
+            static NAME: &str = concat!(stringify!(#name), "\0");
+            static DOC: &str = concat!(#doc, "\0");
+            static MODULE_DEF: ModuleDef = unsafe { ModuleDef::new(NAME, DOC) };
 
-            pyo3::callback::handle_panic(|_py| { MODULE_DEF.make_module(#doc, #fnname) })
+            pyo3::callback::handle_panic(|_py| { MODULE_DEF.make_module(_py, #fnname) })
         }
     }
 }
diff --git a/src/derive_utils.rs b/src/derive_utils.rs
index e784fab5..adc2a80b 100644
--- a/src/derive_utils.rs
+++ b/src/derive_utils.rs
@@ -9,7 +9,7 @@ use crate::exceptions::PyTypeError;
 use crate::instance::PyNativeType;
 use crate::pyclass::PyClass;
 use crate::types::{PyAny, PyDict, PyModule, PyString, PyTuple};
-use crate::{ffi, GILPool, PyCell, Python};
+use crate::{ffi, PyCell, Python};
 use std::cell::UnsafeCell;
 
 #[derive(Debug)]
@@ -281,7 +281,6 @@ pub fn argument_extraction_error(py: Python, arg_name: &str, error: PyErr) -> Py
 }
 
 /// `Sync` wrapper of `ffi::PyModuleDef`.
-#[doc(hidden)]
 pub struct ModuleDef(UnsafeCell<ffi::PyModuleDef>);
 
 unsafe impl Sync for ModuleDef {}
@@ -291,19 +290,29 @@ impl ModuleDef {
     ///
     /// # Safety
     /// `name` must be a null-terminated string.
-    pub const unsafe fn new(name: &'static str) -> Self {
-        #[allow(deprecated)]
-        let mut init = ffi::PyModuleDef_INIT;
-        init.m_name = name.as_ptr() as *const _;
-        ModuleDef(UnsafeCell::new(init))
+    pub const unsafe fn new(name: &'static str, doc: &'static str) -> Self {
+        const INIT: ffi::PyModuleDef = ffi::PyModuleDef {
+            m_base: ffi::PyModuleDef_HEAD_INIT,
+            m_name: std::ptr::null(),
+            m_doc: std::ptr::null(),
+            m_size: 0,
+            m_methods: std::ptr::null_mut(),
+            m_slots: std::ptr::null_mut(),
+            m_traverse: None,
+            m_clear: None,
+            m_free: None,
+        };
+
+        ModuleDef(UnsafeCell::new(ffi::PyModuleDef {
+            m_name: name.as_ptr() as *const _,
+            m_doc: doc.as_ptr() as *const _,
+            ..INIT
+        }))
     }
     /// Builds a module using user given initializer. Used for `#[pymodule]`.
-    ///
-    /// # Safety
-    /// The caller must have GIL.
-    pub unsafe fn make_module(
+    pub fn make_module(
         &'static self,
-        doc: &str,
+        py: Python,
         initializer: impl Fn(Python, &PyModule) -> PyResult<()>,
     ) -> PyResult<*mut ffi::PyObject> {
         #[cfg(py_sys_config = "WITH_THREAD")]
@@ -312,14 +321,8 @@ impl ModuleDef {
         #[cfg(not(Py_3_7))]
         ffi::PyEval_InitThreads();
 
-        let module = ffi::PyModule_Create(self.0.get());
-        let pool = GILPool::new();
-        let py = pool.python();
-        if module.is_null() {
-            return Err(crate::PyErr::fetch(py));
-        }
-        let module = py.from_owned_ptr_or_err::<PyModule>(module)?;
-        module.setattr("__doc__", doc)?;
+        let module =
+            unsafe { py.from_owned_ptr_or_err::<PyModule>(ffi::PyModule_Create(self.0.get()))? };
         initializer(py, module)?;
         Ok(crate::IntoPyPointer::into_ptr(module))
     }
diff --git a/src/ffi/moduleobject.rs b/src/ffi/moduleobject.rs
index d43efa78..96d3e64b 100644
--- a/src/ffi/moduleobject.rs
+++ b/src/ffi/moduleobject.rs
@@ -90,19 +90,3 @@ pub struct PyModuleDef {
     pub m_clear: Option<inquiry>,
     pub m_free: Option<freefunc>,
 }
-
-/// Helper initial value of [`PyModuleDef`] for a Python class.
-///
-/// Not present in the Python C API.
-#[deprecated(note = "not present in Python headers; to be removed")]
-pub const PyModuleDef_INIT: PyModuleDef = PyModuleDef {
-    m_base: PyModuleDef_HEAD_INIT,
-    m_name: std::ptr::null(),
-    m_doc: std::ptr::null(),
-    m_size: 0,
-    m_methods: std::ptr::null_mut(),
-    m_slots: std::ptr::null_mut(),
-    m_traverse: None,
-    m_clear: None,
-    m_free: None,
-};