pyclass: switch from immutable to frozen #2448
Conversation
davidhewitt
force-pushed
the
frozen
branch
4 times, most recently
from
1f7037a to
20324ee
Compare
src/lib.rs
Outdated
| @@ -357,6 +357,7 @@ pub use inventory; // Re-exported for `#[pyclass]` and `#[pymethods]` with `mult | |||
| #[macro_use] | |||
| mod internal_tricks; | |||
|
|
|||
| pub mod boolean_traits; | |||
There was a problem hiding this comment.
I don't think this deserves to be visible as its own file/module. It seems more appropriate to have this (appear to be) a part of src/pyclass.
There was a problem hiding this comment.
I've gone for pyclass::boolean_struct::{True, False} instead.
| error[E0277]: the trait bound `PyDict: PyClass` is not satisfied | ||
| --> tests/ui/abi3_nativetype_inheritance.rs:5:1 | ||
| | | ||
| 5 | #[pyclass(extends=PyDict)] | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `PyClass` is not implemented for `PyDict` | ||
| | | ||
| = note: required because of the requirements on the impl of `PyClassBaseType` for `PyDict` | ||
| note: required by a bound in `PyRefMut` | ||
| --> src/pycell.rs | ||
| | | ||
| | pub struct PyRefMut<'p, T: PyClass<Frozen = False>> { | ||
| | ^^^^^^^^^^^^^^ required by this bound in `PyRefMut` | ||
| = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) | ||
|
|
There was a problem hiding this comment.
Yeah, I fully agree. I'd like to call it out-of-scope of this PR to fix; ultimately I'd like to remove this error completely in #1344
| /// Frozen or not | ||
| type Frozen: Frozen; |
There was a problem hiding this comment.
Given that the True / False nature of this field exactly mirrors the macro attribute, I felt comfortable exposing this in the PyClass trait rather than the PyClassImpl trait. Also seems sensible given that users see this type in the public api docs for e.g. PyRefMut.
There was a problem hiding this comment.
(will merge this branch now, we can always change this before 0.17 if you disagree)
Switches the WIP
#[pyclass(immutable)]to#[pyclass(frozen)], as suggested in #2325 (comment)@mejrs - I had an interesting idea this morning and introduced a "boolean trait" setup which allows us to write
PyClass<Frozen = False>etc. I think it works pretty nicely!(It would have been cool to use an associated const and be able to constrain on
PyClass<FROZEN = false>instead, but associated const equality is unstable. The boolean trait system above seems fine as a replacement.)The only thing remaining with "mutability" in the name is the
PyClassMutabilitytrait, which I consider an implementation detail anyway. I think it should be possible to clean it up later. Just wanted to start by throwing this idea out there.