pyo3 icon indicating copy to clipboard operation
pyo3 copied to clipboard

set_item consumes, but uses ToPyObject instead of IntoPyObject

Open m-ou-se opened this issue 6 years ago • 5 comments

PyDict::set_item consumes the key and value, but uses ToPyObject. Shouldn't that be IntoPyObject?

For ToPyObject, taking a reference would be sufficient. But it seems more efficient to consume and use IntoPyObject.

(Same for PyList::set_item, etc.)

m-ou-se avatar May 11 '19 16:05 m-ou-se

Also set_item borrows mutates the dict. Should it borrow mutable?

PetrGlad avatar Aug 09 '19 18:08 PetrGlad

@PetrGlad No, that's unreleated, and on purpose. Python objects can always have multiple owners, so Rust's mut is not used for mutability in PyO3: See https://docs.rs/pyo3/0.7.0/pyo3/#ownership-and-lifetimes

m-ou-se avatar Aug 11 '19 09:08 m-ou-se

Marking to evaluate this as part of possible collections improvements for 0.15.

davidhewitt avatar Jul 04 '21 21:07 davidhewitt

@davidhewitt What would be the change here? I played a bit (tried changing ToPyObject -> IntoPy<PyObject>) and got into some errors. Is this supposed to be a tricky thing to solve?

aviramha avatar Jan 13 '22 10:01 aviramha

Changing to IntoPy sounds like the right idea, but as it's breaking I have basically been sitting on this as I'm not sure how much it really helps. The semantics of IntoPy and ToPyObject are a little blurred...

davidhewitt avatar Jan 13 '22 23:01 davidhewitt