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

drop could take self by-value, and allow early destruction #4330

Closed
thestinger opened this issue Jan 2, 2013 · 19 comments
Closed

drop could take self by-value, and allow early destruction #4330

thestinger opened this issue Jan 2, 2013 · 19 comments
Labels
A-destructors Area: destructors (Drop, ..) A-typesystem Area: The type system

Comments

@thestinger
Copy link
Contributor

There's currently a util::ignore function that consumes a value to destroy it early.

It would be nicer the if Drop trait's drop method took self by value and could be called early, which would require some compiler magic to avoid an infinite destruction cycle in the drop method. This would also allow drop to move out of self, which is potentially useful.

@erickt
Copy link
Contributor

erickt commented Jan 2, 2013

Now that we can move out of methods, could we just change the finalize method to fn finalize(self)? That'd save us from having to implement another trait.

@thestinger
Copy link
Contributor Author

@erickt: that was actually the original idea, and the idea of doing it with a trait actually came later to avoid needing compiler changes since I hadn't considered that it would result in a destruction cycle. However, it turns out there's already util::ignore which is good enough as a single way to do this, but would still be nice if finalize just took self by-value. Going to update the bug report with that in mind.

@brson
Copy link
Contributor

brson commented Jan 18, 2013

I was thinking this would be useful as well, but I don't know how it can work. During destruction the allocator essentially 'owns' the value and by passing self to finalize it is giving up ownership. This would possibly be ok if the value was trapped in the finalizer and was sure to go out of scope, but the finalizer is also free to send it to another task, or if self is ~str to pass it to fail. There may be other ways it could escape as well.

@nikomatsakis
Copy link
Contributor

I do think that the finalizer should take self "by value". @brson I'm not quite sure what you think would be wrong with that.

@nikomatsakis
Copy link
Contributor

Nominating for milestone 2.

@brson
Copy link
Contributor

brson commented Apr 29, 2013

We discussed at the work week why by-value finalizers are ok and you convinced me at the time, but yet again I don't understand how that works.

@nikomatsakis
Copy link
Contributor

Short version: what we call "by-value" really just means that ownership of the contents is transferred to the callee. So the callee is responsible for freeing the contents, basically. In most cases, the value of a "by-value" parameter is still passed by pointer, it's just that we create a value on our stack and give a pointer to the callee. They are responsible then to free the data in that stack region, we don't free its contents. This seems to match destructors perfectly, if you generalize "stack region" to "memory region".

@brson
Copy link
Contributor

brson commented Apr 29, 2013

It isn't true though that the finalize is responsible for freeing the contents. The compiler generates code to free the contents after finalize finishes. What if finalize moves self somewhere else?

@nikomatsakis
Copy link
Contributor

Yes, today the compiler generates the code to free the contents, but that's precisely what I propose to change. Instead, the contents would be freed (or sent, or moved) by the destructor, just as with any other by-value argument. I don't see a problem with the fact that the destructor can send or move self, in fact, it's one of the motivations for me.

@graydon graydon closed this as completed May 1, 2013
@graydon graydon reopened this May 1, 2013
@graydon
Copy link
Contributor

graydon commented May 1, 2013

oops, closed by accident.

@brson
Copy link
Contributor

brson commented May 2, 2013

@nikomatsakis If the finalizer moves self then does the finalizer run again later or does it remember that it already ran once and never run again?

Here's some code, annotated with what I think you are proposing.

struct Foo;

fn main() {
    let f = Foo;
    let _g = f; // finalize called here
}

impl Drop for Foo {
    fn finalize(self) {
        let _g = self; // finalize not called here?
    }
}

In both those blocks a Foo goes out of scope but in only one the finalizer is called. Expand that to another example:

impl Drop for Foo {
    fn finalize(self) {
        mega_finalize(self);

        fn mega_finalize(f: Foo) {
            let _g = f; // finalize called here?
        }
    }
}

@nikomatsakis
Copy link
Contributor

Hmm, so, I think if you move self as a whole, then it would re-run the finalizer. The only magic thing, I suppose, is that the finalizer does not run on self itself, though it would in other methods that take self by value. What I actually expect people to do is not to move self, but to move parts of self out and send them around.

@nikomatsakis
Copy link
Contributor

We discussed this at the meeting today. Conclusion was that dtors
could take self by-value, but we should have a special rule that
prevents you from moving self all at once.

@nikomatsakis
Copy link
Contributor

On Thu, Jun 27, 2013 at 12:54:01AM -0700, Daniel Micay wrote:

A #[no_drop_flag] attribute has been added to zero the whole
struct on a move, and allow the destructor to be called multiple
times. With &self or &mut self, that should be safe (with safe
code in the destructor) but it won't be with by-value self. It
could be renamed to #[unsafe_no_drop_flag], but it's important to
keep it working because custom smart pointers need it to be
pointer-sized.

There is nothing "safe" about this flag, as far as I can tell? You
might end up with fields of @T or &T type which are null, for
example, which the compiler certainly does not expect. I'm ok with the
flag as an interim step, though naturally I prefer that we just
address this by not requiring the flag in the first place. We have a
plan for doing just this.

@thestinger
Copy link
Contributor Author

@nikomatsakis: yup, it's not actually safe - that's why I deleted the comment :)

@brson
Copy link
Contributor

brson commented Oct 10, 2013

My current opinion as that there are too many special cases required to support by-value finalizers, not enough real use cases, and &mut self is sufficient.

@alexcrichton
Copy link
Member

We decided in today's meeting (https://github.com/mozilla/rust/wiki/Meeting-weekly-2013-10-22) that drop by value requires too many special rules and semantics about it to warrant the convenience of taking self by value instead of by reference. For that reason, I'm de-milestoning the bug, removing the P-backcompat-lang label, and removing the I-completion label.

While we don't plan on doing this for 1.0, there is always the possibility of doing it afterwards. It would be a little painful, but types could implement a DropValue trait if necessary and the compiler would require that both Drop and DropValue are not implemented for the same type. This means that I don't want to close this bug, because it would continue to be an interesting issue to investigate. In the meantime, however, this is not going to be a priority.

@thestinger
Copy link
Contributor Author

I no longer think this is a great idea. It's certainly possible to implement with special cases in the language, but I don't think the additional complexity is worth it. It's easy enough to do anything you could do this way by using unsafe blocks in the destructor, and you will likely need unsafe for most low-level destructors anyway.

@dewert99
Copy link

In case anyone is still interested, I found a crate https://github.com/terrarier2111/owned-drop that enables getting ownership of dropped data without using unsafe (by using unsafe and ManuallyDrop internally).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-destructors Area: destructors (Drop, ..) A-typesystem Area: The type system
Projects
None yet
Development

No branches or pull requests

7 participants