Add error messages for unsupported macro features on compilation (#4194)

* First implementation * tweak error message wording * Fix boolean logic * Remove redundant parens * Add test for weakref error * Fix test * Reword error message * Add expected error output * Rinse and repeat for `dict` * Add test output file * Ignore Rust Rover config files * cargo fmt * Add newsfragment * Update newsfragments/4194.added.md Co-authored-by: David Hewitt <[email protected]> * Use ensure_spanned! macro Co-authored-by: David Hewitt <[email protected]> * Use ensure_spanned! macro for weakref error, too Co-authored-by: David Hewitt <[email protected]> * Revert "Ignore Rust Rover config files" This reverts commit 6c8a2ee. * Update wording for error message Co-authored-by: Icxolu <[email protected]> * Update weakref error message, too * Refactor constant to a pyversions module * Fix compiler errors * Another wording update Co-authored-by: David Hewitt <[email protected]> * And make weakref wording the same * Fix compiler error due to using weakref in our own code * Fix after merge * apply conditional pyclass * update conditional compilation in tests --------- Co-authored-by: cojmeister <[email protected]> Co-authored-by: David Hewitt <[email protected]> Co-authored-by: Icxolu <[email protected]>
PyO3 · Jun 16, 2024 · 79591f2 · 79591f2
1 parent 9648d59
commit 79591f2
Show file tree

Hide file tree

Showing 13 changed files with 151 additions and 92 deletions.
diff --git a/newsfragments/4194.added.md b/newsfragments/4194.added.md
@@ -0,0 +1 @@
+Added error messages when using `weakref` or `dict` when compiling for `abi3` for Python older than 3.9
diff --git a/pyo3-macros-backend/src/lib.rs b/pyo3-macros-backend/src/lib.rs
@@ -19,6 +19,7 @@ mod pyclass;
 mod pyfunction;
 mod pyimpl;
 mod pymethod;
+mod pyversions;
 mod quotes;
 
 pub use frompyobject::build_derive_from_pyobject;

diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs
@@ -1,5 +1,12 @@
 use std::borrow::Cow;
 
+use proc_macro2::{Ident, Span, TokenStream};
+use quote::{format_ident, quote, quote_spanned};
+use syn::ext::IdentExt;
+use syn::parse::{Parse, ParseStream};
+use syn::punctuated::Punctuated;
+use syn::{parse_quote, parse_quote_spanned, spanned::Spanned, Result, Token};
+
 use crate::attributes::kw::frozen;
 use crate::attributes::{
     self, kw, take_pyo3_options, CrateAttribute, ExtendsAttribute, FreelistAttribute,
@@ -14,16 +21,9 @@ use crate::pymethod::{
     impl_py_getter_def, impl_py_setter_def, MethodAndMethodDef, MethodAndSlotDef, PropertyType,
     SlotDef, __GETITEM__, __HASH__, __INT__, __LEN__, __REPR__, __RICHCMP__,
 };
-use crate::utils::Ctx;
 use crate::utils::{self, apply_renaming_rule, PythonDoc};
-use crate::PyFunctionOptions;
-use proc_macro2::{Ident, Span, TokenStream};
-use quote::{format_ident, quote, quote_spanned};
-use syn::ext::IdentExt;
-use syn::parse::{Parse, ParseStream};
-use syn::parse_quote_spanned;
-use syn::punctuated::Punctuated;
-use syn::{parse_quote, spanned::Spanned, Result, Token};
+use crate::utils::{is_abi3, Ctx};
+use crate::{pyversions, PyFunctionOptions};
 
 /// If the class is derived from a Rust `struct` or `enum`.
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
@@ -180,9 +180,17 @@ impl PyClassPyO3Options {
             };
         }
 
+        let python_version = pyo3_build_config::get().version;
+
         match option {
             PyClassPyO3Option::Crate(krate) => set_option!(krate),
-            PyClassPyO3Option::Dict(dict) => set_option!(dict),
+            PyClassPyO3Option::Dict(dict) => {
+                ensure_spanned!(
+                    python_version >= pyversions::PY_3_9 || !is_abi3(),
+                    dict.span() => "`dict` requires Python >= 3.9 when using the `abi3` feature"
+                );
+                set_option!(dict);
+            }
             PyClassPyO3Option::Eq(eq) => set_option!(eq),
             PyClassPyO3Option::EqInt(eq_int) => set_option!(eq_int),
             PyClassPyO3Option::Extends(extends) => set_option!(extends),
@@ -199,7 +207,13 @@ impl PyClassPyO3Options {
             PyClassPyO3Option::SetAll(set_all) => set_option!(set_all),
             PyClassPyO3Option::Subclass(subclass) => set_option!(subclass),
             PyClassPyO3Option::Unsendable(unsendable) => set_option!(unsendable),
-            PyClassPyO3Option::Weakref(weakref) => set_option!(weakref),
+            PyClassPyO3Option::Weakref(weakref) => {
+                ensure_spanned!(
+                    python_version >= pyversions::PY_3_9 || !is_abi3(),
+                    weakref.span() => "`weakref` requires Python >= 3.9 when using the `abi3` feature"
+                );
+                set_option!(weakref);
+            }
         }
         Ok(())
     }

diff --git a/pyo3-macros-backend/src/pyversions.rs b/pyo3-macros-backend/src/pyversions.rs
@@ -0,0 +1,3 @@
+use pyo3_build_config::PythonVersion;
+
+pub const PY_3_9: PythonVersion = PythonVersion { major: 3, minor: 9 };
diff --git a/src/tests/hygiene/pyclass.rs b/src/tests/hygiene/pyclass.rs
@@ -10,15 +10,24 @@ pub struct Foo;
 #[pyo3(crate = "crate")]
 pub struct Foo2;
 
-#[crate::pyclass(
+#[cfg_attr(any(Py_3_9, not(Py_LIMITED_API)), crate::pyclass(
     name = "ActuallyBar",
     freelist = 8,
+    unsendable,
+    subclass,
+    extends = crate::types::PyAny,
+    module = "Spam",
     weakref,
+    dict
+))]
+#[cfg_attr(not(any(Py_3_9, not(Py_LIMITED_API))), crate::pyclass(
+    name = "ActuallyBar",
+    freelist = 8,
     unsendable,
     subclass,
     extends = crate::types::PyAny,
     module = "Spam"
-)]
+))]
 #[pyo3(crate = "crate")]
 pub struct Bar {
     #[pyo3(get, set)]

diff --git a/tests/test_class_basics.rs b/tests/test_class_basics.rs
@@ -428,6 +428,7 @@ fn test_tuple_struct_class() {
     });
 }
 
+#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
 #[pyclass(dict, subclass)]
 struct DunderDictSupport {
     // Make sure that dict_offset runs with non-zero sized Self
@@ -479,6 +480,7 @@ fn access_dunder_dict() {
 }
 
 // If the base class has dict support, child class also has dict
+#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
 #[pyclass(extends=DunderDictSupport)]
 struct InheritDict {
     _value: usize,
@@ -509,6 +511,7 @@ fn inherited_dict() {
     });
 }
 
+#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
 #[pyclass(weakref, dict)]
 struct WeakRefDunderDictSupport {
     // Make sure that weaklist_offset runs with non-zero sized Self
@@ -534,6 +537,7 @@ fn weakref_dunder_dict_support() {
     });
 }
 
+#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
 #[pyclass(weakref, subclass)]
 struct WeakRefSupport {
     _pad: [u8; 32],
@@ -559,6 +563,7 @@ fn weakref_support() {
 }
 
 // If the base class has weakref support, child class also has weakref.
+#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
 #[pyclass(extends=WeakRefSupport)]
 struct InheritWeakRef {
     _value: usize,
@@ -666,3 +671,48 @@ fn drop_unsendable_elsewhere() {
         capture.borrow_mut(py).uninstall(py);
     });
 }
+
+#[test]
+#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
+fn test_unsendable_dict() {
+    #[pyclass(dict, unsendable)]
+    struct UnsendableDictClass {}
+
+    #[pymethods]
+    impl UnsendableDictClass {
+        #[new]
+        fn new() -> Self {
+            UnsendableDictClass {}
+        }
+    }
+
+    Python::with_gil(|py| {
+        let inst = Py::new(py, UnsendableDictClass {}).unwrap();
+        py_run!(py, inst, "assert inst.__dict__ == {}");
+    });
+}
+
+#[test]
+#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
+fn test_unsendable_dict_with_weakref() {
+    #[pyclass(dict, unsendable, weakref)]
+    struct UnsendableDictClassWithWeakRef {}
+
+    #[pymethods]
+    impl UnsendableDictClassWithWeakRef {
+        #[new]
+        fn new() -> Self {
+            Self {}
+        }
+    }
+
+    Python::with_gil(|py| {
+        let inst = Py::new(py, UnsendableDictClassWithWeakRef {}).unwrap();
+        py_run!(py, inst, "assert inst.__dict__ == {}");
+        py_run!(
+            py,
+            inst,
+            "import weakref; assert weakref.ref(inst)() is inst; inst.a = 1; assert inst.a == 1"
+        );
+    });
+}
diff --git a/tests/test_compile_error.rs b/tests/test_compile_error.rs
@@ -67,4 +67,8 @@ fn test_compile_errors() {
     #[cfg(all(Py_LIMITED_API, not(feature = "experimental-async")))]
     // output changes with async feature
     t.compile_fail("tests/ui/abi3_inheritance.rs");
+    #[cfg(all(Py_LIMITED_API, not(Py_3_9)))]
+    t.compile_fail("tests/ui/abi3_weakref.rs");
+    #[cfg(all(Py_LIMITED_API, not(Py_3_9)))]
+    t.compile_fail("tests/ui/abi3_dict.rs");
 }
diff --git a/tests/test_unsendable_dict.rs b/tests/test_unsendable_dict.rs
diff --git a/tests/test_various.rs b/tests/test_various.rs
@@ -2,7 +2,7 @@
 
 use pyo3::prelude::*;
 use pyo3::py_run;
-use pyo3::types::{PyDict, PyTuple};
+use pyo3::types::PyTuple;
 
 use std::fmt;
 
@@ -113,39 +113,41 @@ fn pytuple_pyclass_iter() {
     });
 }
 
-#[pyclass(dict, module = "test_module")]
-struct PickleSupport {}
-
-#[pymethods]
-impl PickleSupport {
-    #[new]
-    fn new() -> PickleSupport {
-        PickleSupport {}
+#[test]
+#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
+fn test_pickle() {
+    use pyo3::types::PyDict;
+
+    #[pyclass(dict, module = "test_module")]
+    struct PickleSupport {}
+
+    #[pymethods]
+    impl PickleSupport {
+        #[new]
+        fn new() -> PickleSupport {
+            PickleSupport {}
+        }
+
+        pub fn __reduce__<'py>(
+            slf: &Bound<'py, Self>,
+            py: Python<'py>,
+        ) -> PyResult<(PyObject, Bound<'py, PyTuple>, PyObject)> {
+            let cls = slf.to_object(py).getattr(py, "__class__")?;
+            let dict = slf.to_object(py).getattr(py, "__dict__")?;
+            Ok((cls, PyTuple::empty_bound(py), dict))
+        }
     }
 
-    pub fn __reduce__<'py>(
-        slf: &Bound<'py, Self>,
-        py: Python<'py>,
-    ) -> PyResult<(PyObject, Bound<'py, PyTuple>, PyObject)> {
-        let cls = slf.to_object(py).getattr(py, "__class__")?;
-        let dict = slf.to_object(py).getattr(py, "__dict__")?;
-        Ok((cls, PyTuple::empty_bound(py), dict))
+    fn add_module(module: Bound<'_, PyModule>) -> PyResult<()> {
+        PyModule::import_bound(module.py(), "sys")?
+            .dict()
+            .get_item("modules")
+            .unwrap()
+            .unwrap()
+            .downcast::<PyDict>()?
+            .set_item(module.name()?, module)
     }
-}
 
-fn add_module(module: Bound<'_, PyModule>) -> PyResult<()> {
-    PyModule::import_bound(module.py(), "sys")?
-        .dict()
-        .get_item("modules")
-        .unwrap()
-        .unwrap()
-        .downcast::<PyDict>()?
-        .set_item(module.name()?, module)
-}
-
-#[test]
-#[cfg_attr(all(Py_LIMITED_API, not(Py_3_10)), ignore)]
-fn test_pickle() {
     Python::with_gil(|py| {
         let module = PyModule::new_bound(py, "test_module").unwrap();
         module.add_class::<PickleSupport>().unwrap();

diff --git a/tests/ui/abi3_dict.rs b/tests/ui/abi3_dict.rs
@@ -0,0 +1,7 @@
+//! With abi3, dict not supported until python 3.9 or greater
+use pyo3::prelude::*;
+
+#[pyclass(dict)]
+struct TestClass {}
+
+fn main() {}
diff --git a/tests/ui/abi3_dict.stderr b/tests/ui/abi3_dict.stderr
@@ -0,0 +1,5 @@
+error: `dict` requires Python >= 3.9 when using the `abi3` feature
+ --> tests/ui/abi3_dict.rs:4:11
+  |
+4 | #[pyclass(dict)]
+  |           ^^^^
diff --git a/tests/ui/abi3_weakref.rs b/tests/ui/abi3_weakref.rs
@@ -0,0 +1,7 @@
+//! With abi3, weakref not supported until python 3.9 or greater
+use pyo3::prelude::*;
+
+#[pyclass(weakref)]
+struct TestClass {}
+
+fn main() {}
diff --git a/tests/ui/abi3_weakref.stderr b/tests/ui/abi3_weakref.stderr
@@ -0,0 +1,5 @@
+error: `weakref` requires Python >= 3.9 when using the `abi3` feature
+ --> tests/ui/abi3_weakref.rs:4:11
+  |
+4 | #[pyclass(weakref)]
+  |           ^^^^^^^