Skip to content

Commit

Permalink
pyclass: add sequence option to implement sq_length
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Aug 19, 2022
1 parent 5d44c52 commit fd5d682
Show file tree
Hide file tree
Showing 11 changed files with 69 additions and 22 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Add support for generating PyPy Windows import library. [#2506](https://github.com/PyO3/pyo3/pull/2506)
- Add FFI definitions for `Py_EnterRecursiveCall` and `Py_LeaveRecursiveCall`. [#2511](https://github.com/PyO3/pyo3/pull/2511)
- Add `get_item_with_error` on `PyDict` that exposes `PyDict_GetItemWIthError` for non-PyPy. [#2536](https://github.com/PyO3/pyo3/pull/2536)
- Add `#[pyclass(sequence)]` option. [#2567](https://github.com/PyO3/pyo3/pull/2567)

### Changed

Expand Down
6 changes: 4 additions & 2 deletions guide/pyclass_parameters.md
Expand Up @@ -10,8 +10,9 @@
| `mapping` | Inform PyO3 that this class is a [`Mapping`][params-mapping], and so leave its implementation of sequence C-API slots empty. |
| <span style="white-space: pre">`module = "module_name"`</span> | Python code will see the class as being defined in this module. Defaults to `builtins`. |
| <span style="white-space: pre">`name = "python_name"`</span> | Sets the name that Python sees this class as. Defaults to the name of the Rust struct. |
| <span style="white-space: pre">`text_signature = "(arg1, arg2, ...)"`</span> | Sets the text signature for the Python class' `__new__` method. |
| `sequence` | Inform PyO3 that this class is a [`Sequence`][params-sequence], and so leave its C-API mapping length slot empty. |
| `subclass` | Allows other Python classes and `#[pyclass]` to inherit from this class. Enums cannot be subclassed. |
| <span style="white-space: pre">`text_signature = "(arg1, arg2, ...)"`</span> | Sets the text signature for the Python class' `__new__` method. |
| `unsendable` | Required if your struct is not [`Send`][params-3]. Rather than using `unsendable`, consider implementing your struct in a threadsafe way by e.g. substituting [`Rc`][params-4] with [`Arc`][params-5]. By using `unsendable`, your class will panic when accessed by another thread.|
| `weakref` | Allows this class to be [weakly referenceable][params-6]. |

Expand All @@ -35,4 +36,5 @@ struct MyClass { }
[params-4]: https://doc.rust-lang.org/std/rc/struct.Rc.html
[params-5]: https://doc.rust-lang.org/std/sync/struct.Arc.html
[params-6]: https://docs.python.org/3/library/weakref.html
[params-mapping]: https://pyo3.rs/latest/class/protocols.html#mapping--sequence-types
[params-mapping]: https://pyo3.rs/latest/class/protocols.html#mapping--sequence-types
[params-sequence]: https://pyo3.rs/latest/class/protocols.html#mapping--sequence-types
4 changes: 3 additions & 1 deletion guide/src/class/protocols.md
Expand Up @@ -207,9 +207,11 @@ Mapping types usually will not want the sequence slots filled. Having them fille

Use the `#[pyclass(mapping)]` annotation to instruct PyO3 to only fill the mapping slots, leaving the sequence ones empty. This will apply to `__getitem__`, `__setitem__`, and `__delitem__`.

Use the `#[pyclass(sequence)]` annotation to instruct PyO3 to fill the `sq_length` slot instead of the `mp_length` slot for `__len__`. This will help libraries such as `numpy` recognise the class as a sequence, however will also cause CPython to automatically add the sequence length to any negative indices before passing them to `__getitem__`. (`__getitem__`, `__setitem__` and `__delitem__` mapping slots are still used for sequences, for slice operations.)

- `__len__(<self>) -> usize`

Implements the built-in function `len()` for the sequence.
Implements the built-in function `len()`.

- `__contains__(<self>, object) -> bool`

Expand Down
4 changes: 2 additions & 2 deletions guide/src/migration.md
Expand Up @@ -230,9 +230,9 @@ class ExampleContainer:

This class implements a Python [sequence](https://docs.python.org/3/glossary.html#term-sequence).

The `__len__` and `__getitem__` methods are also used to implement a Python [mapping](https://docs.python.org/3/glossary.html#term-mapping). In the Python C-API, these methods are not shared: the sequence `__len__` and `__getitem__` are defined by the `sq_len` and `sq_item` slots, and the mapping equivalents are `mp_len` and `mp_subscript`. There are similar distinctions for `__setitem__` and `__delitem__`.
The `__len__` and `__getitem__` methods are also used to implement a Python [mapping](https://docs.python.org/3/glossary.html#term-mapping). In the Python C-API, these methods are not shared: the sequence `__len__` and `__getitem__` are defined by the `sq_length` and `sq_item` slots, and the mapping equivalents are `mp_length` and `mp_subscript`. There are similar distinctions for `__setitem__` and `__delitem__`.

Because there is no such distinction from Python, implementing these methods will fill the mapping and sequence slots simultaneously. A Python class with `__len__` implemented, for example, will have both the `sq_len` and `mp_len` slots filled.
Because there is no such distinction from Python, implementing these methods will fill the mapping and sequence slots simultaneously. A Python class with `__len__` implemented, for example, will have both the `sq_length` and `mp_length` slots filled.

The PyO3 behavior in 0.16 has been changed to be closer to this Python behavior by default.

Expand Down
1 change: 1 addition & 0 deletions pyo3-macros-backend/src/attributes.rs
Expand Up @@ -23,6 +23,7 @@ pub mod kw {
syn::custom_keyword!(module);
syn::custom_keyword!(name);
syn::custom_keyword!(pass_module);
syn::custom_keyword!(sequence);
syn::custom_keyword!(set);
syn::custom_keyword!(signature);
syn::custom_keyword!(subclass);
Expand Down
41 changes: 27 additions & 14 deletions pyo3-macros-backend/src/pyclass.rs
Expand Up @@ -65,6 +65,7 @@ pub struct PyClassPyO3Options {
pub mapping: Option<kw::mapping>,
pub module: Option<ModuleAttribute>,
pub name: Option<NameAttribute>,
pub sequence: Option<kw::sequence>,
pub subclass: Option<kw::subclass>,
pub text_signature: Option<TextSignatureAttribute>,
pub unsendable: Option<kw::unsendable>,
Expand All @@ -82,6 +83,7 @@ enum PyClassPyO3Option {
Mapping(kw::mapping),
Module(ModuleAttribute),
Name(NameAttribute),
Sequence(kw::sequence),
Subclass(kw::subclass),
TextSignature(TextSignatureAttribute),
Unsendable(kw::unsendable),
Expand Down Expand Up @@ -109,6 +111,8 @@ impl Parse for PyClassPyO3Option {
input.parse().map(PyClassPyO3Option::Module)
} else if lookahead.peek(kw::name) {
input.parse().map(PyClassPyO3Option::Name)
} else if lookahead.peek(attributes::kw::sequence) {
input.parse().map(PyClassPyO3Option::Sequence)
} else if lookahead.peek(attributes::kw::subclass) {
input.parse().map(PyClassPyO3Option::Subclass)
} else if lookahead.peek(attributes::kw::text_signature) {
Expand Down Expand Up @@ -164,6 +168,7 @@ impl PyClassPyO3Options {
PyClassPyO3Option::Mapping(mapping) => set_option!(mapping),
PyClassPyO3Option::Module(module) => set_option!(module),
PyClassPyO3Option::Name(name) => set_option!(name),
PyClassPyO3Option::Sequence(sequence) => set_option!(sequence),
PyClassPyO3Option::Subclass(subclass) => set_option!(subclass),
PyClassPyO3Option::TextSignature(text_signature) => set_option!(text_signature),
PyClassPyO3Option::Unsendable(unsendable) => set_option!(unsendable),
Expand Down Expand Up @@ -315,7 +320,7 @@ fn impl_class(
vec![],
)
.doc(doc)
.impl_all();
.impl_all()?;

Ok(quote! {
const _: () = {
Expand Down Expand Up @@ -414,7 +419,7 @@ pub fn build_py_enum(
.map(|attr| (get_class_python_name(&enum_.ident, &args), attr)),
);
let enum_ = PyClassEnum::new(enum_)?;
Ok(impl_enum(enum_, &args, doc, method_type))
impl_enum(enum_, &args, doc, method_type)
}

/// `#[pyo3()]` options for pyclass enum variants
Expand Down Expand Up @@ -462,7 +467,7 @@ fn impl_enum(
args: &PyClassArgs,
doc: PythonDoc,
methods_type: PyClassMethodsType,
) -> TokenStream {
) -> Result<TokenStream> {
let krate = get_pyo3_crate(&args.options.krate);
impl_enum_class(enum_, args, doc, methods_type, krate)
}
Expand All @@ -473,7 +478,7 @@ fn impl_enum_class(
doc: PythonDoc,
methods_type: PyClassMethodsType,
krate: syn::Path,
) -> TokenStream {
) -> Result<TokenStream> {
let cls = enum_.ident;
let ty: syn::Type = syn::parse_quote!(#cls);
let variants = enum_.variants;
Expand Down Expand Up @@ -569,9 +574,9 @@ fn impl_enum_class(
default_slots,
)
.doc(doc)
.impl_all();
.impl_all()?;

quote! {
Ok(quote! {
const _: () = {
use #krate as _pyo3;

Expand All @@ -587,7 +592,7 @@ fn impl_enum_class(
#default_richcmp
}
};
}
})
}

fn generate_default_protocol_slot(
Expand Down Expand Up @@ -752,16 +757,17 @@ impl<'a> PyClassImplsBuilder<'a> {
}
}

fn impl_all(&self) -> TokenStream {
vec![
fn impl_all(&self) -> Result<TokenStream> {
let tokens = vec![
self.impl_pyclass(),
self.impl_extractext(),
self.impl_into_py(),
self.impl_pyclassimpl(),
self.impl_pyclassimpl()?,
self.impl_freelist(),
]
.into_iter()
.collect()
.collect();
Ok(tokens)
}

fn impl_pyclass(&self) -> TokenStream {
Expand Down Expand Up @@ -828,7 +834,7 @@ impl<'a> PyClassImplsBuilder<'a> {
quote! {}
}
}
fn impl_pyclassimpl(&self) -> TokenStream {
fn impl_pyclassimpl(&self) -> Result<TokenStream> {
let cls = self.cls;
let doc = self.doc.as_ref().map_or(quote! {"\0"}, |doc| quote! {#doc});
let is_basetype = self.attr.options.subclass.is_some();
Expand All @@ -841,6 +847,12 @@ impl<'a> PyClassImplsBuilder<'a> {
.unwrap_or_else(|| parse_quote! { _pyo3::PyAny });
let is_subclass = self.attr.options.extends.is_some();
let is_mapping: bool = self.attr.options.mapping.is_some();
let is_sequence: bool = self.attr.options.sequence.is_some();

ensure_spanned!(
!(is_mapping && is_sequence),
self.cls.span() => "a `#[pyclass]` cannot be both a `mapping` and a `sequence`"
);

let dict_offset = if self.attr.options.dict.is_some() {
quote! {
Expand Down Expand Up @@ -959,12 +971,13 @@ impl<'a> PyClassImplsBuilder<'a> {
quote! { _pyo3::PyAny }
};

quote! {
Ok(quote! {
impl _pyo3::impl_::pyclass::PyClassImpl for #cls {
const DOC: &'static str = #doc;
const IS_BASETYPE: bool = #is_basetype;
const IS_SUBCLASS: bool = #is_subclass;
const IS_MAPPING: bool = #is_mapping;
const IS_SEQUENCE: bool = #is_sequence;

type Layout = _pyo3::PyCell<Self>;
type BaseType = #base;
Expand Down Expand Up @@ -1002,7 +1015,7 @@ impl<'a> PyClassImplsBuilder<'a> {
}

#inventory_class
}
})
}

fn impl_freelist(&self) -> TokenStream {
Expand Down
3 changes: 3 additions & 0 deletions src/impl_/pyclass.rs
Expand Up @@ -146,6 +146,9 @@ pub trait PyClassImpl: Sized {
/// #[pyclass(mapping)]
const IS_MAPPING: bool = false;

/// #[pyclass(sequence)]
const IS_SEQUENCE: bool = false;

/// Layout
type Layout: PyLayout<Self>;

Expand Down
16 changes: 16 additions & 0 deletions src/pyclass.rs
Expand Up @@ -43,6 +43,7 @@ where
.slot(ffi::Py_tp_dealloc, tp_dealloc::<T> as *mut c_void)
.set_is_basetype(T::IS_BASETYPE)
.set_is_mapping(T::IS_MAPPING)
.set_is_sequence(T::IS_SEQUENCE)
.class_items(T::items_iter())
.build(py, T::NAME, T::MODULE, std::mem::size_of::<T::Layout>())
} {
Expand All @@ -63,6 +64,7 @@ struct PyTypeBuilder {
/// except for that it does and we have tests.
cleanup: Vec<PyTypeBuilderCleanup>,
is_mapping: bool,
is_sequence: bool,
has_new: bool,
has_dealloc: bool,
has_getitem: bool,
Expand Down Expand Up @@ -225,6 +227,11 @@ impl PyTypeBuilder {
self
}

fn set_is_sequence(mut self, is_sequence: bool) -> Self {
self.is_sequence = is_sequence;
self
}

/// # Safety
/// All slots in the PyClassItemsIter should be correct
unsafe fn class_items(mut self, iter: PyClassItemsIter) -> Self {
Expand Down Expand Up @@ -348,6 +355,15 @@ impl PyTypeBuilder {
)));
}

// For sequences, implement sq_length instead of mp_length
if self.is_sequence {
for slot in &mut self.slots {
if slot.slot == ffi::Py_mp_length {
slot.slot = ffi::Py_sq_length;
}
}
}

// Add empty sentinel at the end
// Safety: python expects this empty slot
unsafe { self.push_slot(0, ptr::null_mut::<c_void>()) }
Expand Down
2 changes: 1 addition & 1 deletion tests/test_sequence.rs
Expand Up @@ -279,7 +279,7 @@ fn test_generic_list_set() {
});
}

#[pyclass]
#[pyclass(sequence)]
struct OptionList {
#[pyo3(get, set)]
items: Vec<Option<i64>>,
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/invalid_pyclass_args.rs
Expand Up @@ -21,4 +21,7 @@ struct InvalidModule {}
#[pyclass(weakrev)]
struct InvalidArg {}

#[pyclass(mapping, sequence)]
struct CannotBeMappingAndSequence {}

fn main() {}
10 changes: 8 additions & 2 deletions tests/ui/invalid_pyclass_args.stderr
@@ -1,4 +1,4 @@
error: expected one of: `crate`, `dict`, `extends`, `freelist`, `frozen`, `mapping`, `module`, `name`, `subclass`, `text_signature`, `unsendable`, `weakref`, `gc`
error: expected one of: `crate`, `dict`, `extends`, `freelist`, `frozen`, `mapping`, `module`, `name`, `sequence`, `subclass`, `text_signature`, `unsendable`, `weakref`, `gc`
--> tests/ui/invalid_pyclass_args.rs:3:11
|
3 | #[pyclass(extend=pyo3::types::PyDict)]
Expand Down Expand Up @@ -34,8 +34,14 @@ error: expected string literal
18 | #[pyclass(module = my_module)]
| ^^^^^^^^^

error: expected one of: `crate`, `dict`, `extends`, `freelist`, `frozen`, `mapping`, `module`, `name`, `subclass`, `text_signature`, `unsendable`, `weakref`, `gc`
error: expected one of: `crate`, `dict`, `extends`, `freelist`, `frozen`, `mapping`, `module`, `name`, `sequence`, `subclass`, `text_signature`, `unsendable`, `weakref`, `gc`
--> tests/ui/invalid_pyclass_args.rs:21:11
|
21 | #[pyclass(weakrev)]
| ^^^^^^^

error: a `#[pyclass]` cannot be both a `mapping` and a `sequence`
--> tests/ui/invalid_pyclass_args.rs:25:8
|
25 | struct CannotBeMappingAndSequence {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

0 comments on commit fd5d682

Please sign in to comment.