Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add error messages for unsupported macro features on compilation #4194

Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
ffa283f
First implementation
cojmeister Mar 23, 2024
61c488f
tweak error message wording
codeguru42 May 20, 2024
2dda39c
Fix boolean logic
codeguru42 May 20, 2024
f1f3efb
Remove redundant parens
codeguru42 May 20, 2024
e4719c3
Add test for weakref error
codeguru42 May 20, 2024
0e25b7c
Fix test
codeguru42 May 20, 2024
36b28d2
Reword error message
codeguru42 May 20, 2024
bfa8cb6
Add expected error output
codeguru42 May 20, 2024
219202f
Rinse and repeat for `dict`
codeguru42 May 20, 2024
03c2cdc
Add test output file
codeguru42 May 20, 2024
2df737c
Ignore Rust Rover config files
codeguru42 May 20, 2024
08891c9
cargo fmt
codeguru42 May 20, 2024
af3d93d
Add newsfragment
codeguru42 May 20, 2024
e22a51a
Update newsfragments/4194.added.md
codeguru42 May 22, 2024
aaf6fa7
Use ensure_spanned! macro
codeguru42 May 22, 2024
0336aeb
Use ensure_spanned! macro for weakref error, too
codeguru42 May 22, 2024
c5dcad8
Revert "Ignore Rust Rover config files"
codeguru42 May 22, 2024
6e83437
Update wording for error message
codeguru42 May 25, 2024
490cee4
Update weakref error message, too
codeguru42 May 25, 2024
9fb5196
Refactor constant to a pyversions module
codeguru42 May 25, 2024
5253cf4
Fix compiler errors
codeguru42 May 25, 2024
a2a9452
Another wording update
codeguru42 May 26, 2024
77a9101
And make weakref wording the same
codeguru42 May 26, 2024
28cbbc6
Fix compiler error due to using weakref in our own code
codeguru42 May 26, 2024
abd1075
Merge remote-tracking branch 'refs/remotes/upstream/main' into add-er…
codeguru42 Jun 4, 2024
680f471
Fix after merge
codeguru42 Jun 4, 2024
4fbbe98
apply conditional pyclass
davidhewitt Jun 6, 2024
7177d53
update conditional compilation in tests
davidhewitt Jun 6, 2024
3c7e898
Merge remote-tracking branch 'refs/remotes/upstream/main' into add-er…
codeguru42 Jun 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/4194.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added error messages when using `weakref` or `dict` when compiling for `abi3` for Python older than 3.9
1 change: 1 addition & 0 deletions pyo3-macros-backend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ mod pyclass;
mod pyfunction;
mod pyimpl;
mod pymethod;
mod pyversions;
mod quotes;

pub use frompyobject::build_derive_from_pyobject;
Expand Down
36 changes: 25 additions & 11 deletions pyo3-macros-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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)]
Expand Down Expand Up @@ -176,9 +176,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),
Expand All @@ -194,7 +202,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(())
}
Expand Down
3 changes: 3 additions & 0 deletions pyo3-macros-backend/src/pyversions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
use pyo3_build_config::PythonVersion;

pub const PY_3_9: PythonVersion = PythonVersion { major: 3, minor: 9 };
13 changes: 11 additions & 2 deletions src/tests/hygiene/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
62 changes: 56 additions & 6 deletions tests/test_class_basics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,14 +428,15 @@ 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
_pad: [u8; 32],
}

#[test]
#[cfg_attr(all(Py_LIMITED_API, not(Py_3_9)), ignore)]
#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
fn dunder_dict_support() {
Python::with_gil(|py| {
let inst = Py::new(
Expand All @@ -458,7 +459,7 @@ fn dunder_dict_support() {

// Accessing inst.__dict__ only supported in limited API from Python 3.10
#[test]
#[cfg_attr(all(Py_LIMITED_API, not(Py_3_10)), ignore)]
#[cfg(any(Py_3_10, not(Py_LIMITED_API)))]
fn access_dunder_dict() {
Python::with_gil(|py| {
let inst = Py::new(
Expand All @@ -480,13 +481,14 @@ 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,
}

#[test]
#[cfg_attr(all(Py_LIMITED_API, not(Py_3_9)), ignore)]
#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
fn inherited_dict() {
Python::with_gil(|py| {
let inst = Py::new(
Expand All @@ -510,14 +512,15 @@ 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
_pad: [u8; 32],
}

#[test]
#[cfg_attr(all(Py_LIMITED_API, not(Py_3_9)), ignore)]
#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
fn weakref_dunder_dict_support() {
Python::with_gil(|py| {
let inst = Py::new(
Expand All @@ -535,13 +538,14 @@ fn weakref_dunder_dict_support() {
});
}

#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
#[pyclass(weakref, subclass)]
struct WeakRefSupport {
_pad: [u8; 32],
}

#[test]
#[cfg_attr(all(Py_LIMITED_API, not(Py_3_9)), ignore)]
#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
fn weakref_support() {
Python::with_gil(|py| {
let inst = Py::new(
Expand All @@ -560,13 +564,14 @@ 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,
}

#[test]
#[cfg_attr(all(Py_LIMITED_API, not(Py_3_9)), ignore)]
#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
fn inherited_weakref() {
Python::with_gil(|py| {
let inst = Py::new(
Expand Down Expand Up @@ -667,3 +672,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"
);
});
}
4 changes: 4 additions & 0 deletions tests/test_compile_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
49 changes: 0 additions & 49 deletions tests/test_unsendable_dict.rs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged these tests into test_class_basics.

This file was deleted.

62 changes: 32 additions & 30 deletions tests/test_various.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use pyo3::prelude::*;
use pyo3::py_run;
use pyo3::types::{PyDict, PyTuple};
use pyo3::types::PyTuple;

use std::fmt;

Expand Down Expand Up @@ -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();
Expand Down
7 changes: 7 additions & 0 deletions tests/ui/abi3_dict.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
//! With abi3, dict not supported until python 3.9 or greater
use pyo3::prelude::*;

#[pyclass(dict)]
struct TestClass {}

fn main() {}
5 changes: 5 additions & 0 deletions tests/ui/abi3_dict.stderr
Original file line number Diff line number Diff line change
@@ -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)]
| ^^^^
Loading
Loading