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

OnMove trait is much more general than Pin #2613

Closed
xialvjun opened this issue Dec 15, 2018 · 5 comments
Closed

OnMove trait is much more general than Pin #2613

xialvjun opened this issue Dec 15, 2018 · 5 comments

Comments

@xialvjun
Copy link

We use Pin to support self referencing type.

But what if we have a OnMove trait which define a method fn on_move(&mut self). Then when we move an object, on_move is called automatically.

OnMove trait is just like Drop trait, it's a part of the lifecycle of an object.

To make the name OnMove and Drop be the same pattern, we can name it Move.

Just an idea. If it's wrong, closed it.

@F001
Copy link
Contributor

F001 commented Dec 15, 2018

Please read previous discussions on Pin. Not having move-constructor/move-assignment operator is a huge advantage of rust.

@xialvjun
Copy link
Author

@F001 Can you give a link?
In https://github.com/rust-lang/rfcs/blob/master/text/2349-pin.md I see Move. But it has different concept than OnMove. Move is just a marker trait, OnMove is a lifecycle trait.
I searched move-constructor rust in google and get https://doc.rust-lang.org/nightly/nomicon/constructors.html. Hope for more document or discussion about it.

@F001
Copy link
Contributor

F001 commented Dec 15, 2018

You can search in internals.rust-lang.org, for example:
https://internals.rust-lang.org/t/idea-limited-custom-move-semantics-through-explicitly-specified-relocations/6704/2
https://internals.rust-lang.org/t/implicit-buffers-overloading-the-assignment-operator/8751/3
https://internals.rust-lang.org/t/idea-on-movable-self-referential-structs/6694

Pin related issues have been discussed for more than a year, I believe all the possibilities have been explored. Repeating deprecated ideas does not help.

@Ixrec
Copy link
Contributor

Ixrec commented Dec 15, 2018

In this case I think the question is reasonable, yet extremely difficult to get a straight answer to just from searching past discussions, since there's so many more issues overlapping here than it might seem. So here's my high-level summary of why we went with Pin:

  • Move constructors in general don't provide enough benefits to offset their cost. In particular, since Rust has had move semantics as the default from day one, the vast majority of C++'s motivation for adding move constructors simply doesn't apply to Rust. @F001's links cover this pretty well.

  • There's no way to make move constructors work without a new implicit trait bound. As far as I know, this is true of every proposed variation we've seen, including your OnMove. Implicit trait bounds run into all of the problems discussed here: More implicit bounds (?Sized, ?DynSized, ?Move) #2255 But to save you the time reading all of that and its many additional links...

    • Adding a new implicit trait bound is (surprise!) backwards incompatible. And this is not the kind of backwards incompatibility that editions let us get away with. So it turns out that was never an option anyway.

    • Solving this via a trait bound makes borrowing across yield points into "everyone's problem", i.e. you have to explicitly make it work or say you don't want to make it work, even when that feature is completely irrelevant to the type you're trying to write. So even if this was pre-Rust 1.0, we'd probably still want Pin because that localizes the solution to code that actually needs to care about the problem.

@SimonSapin
Copy link
Contributor

Also, there’s a lot of unsafe code out there that implements moves as a shallow byte copy. For example, call pushing to a Vec that is at capacity calls realloc. Declaring such code incorrect because it fails to call on_move manually (which did not exist at the time the code was written) is too big of a breaking change, even for editions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants