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

Add doc example for std::ptr::drop_in_place. #34888

Closed
wants to merge 1 commit into from

Conversation

frewsxcv
Copy link
Member

No description provided.

@frewsxcv
Copy link
Member Author

I'm not convinced this is the best example, so if someone has a suggestion for improvement here, let me know!

@eefriedman
Copy link
Contributor

This is a really terrible example. The biggest problem is that the behavior will change once we transition to MIR and get rid of the old drop flag infrastructure. The other issue is that it's not clear what the code is expected to do without actually running it; using assert! is usually better.

A good example would probably involve using malloc() and ptr::write().

@frewsxcv
Copy link
Member Author

@eefriedman Thanks for your feedback!

The biggest problem is that the behavior will change once we transition to MIR and get rid of the old drop flag infrastructure.

Will this example run differently once the transition to MIR happens? If so, in what ways?

The other issue is that it's not clear what the code is expected to do without actually running it; using assert! is usually better.

How about something like this?

use std::ptr::drop_in_place;

struct SomeStruct {
    pub destructor_ran: bool,
};

impl Drop for SomeStruct {
    fn drop(&mut self) {
        self.destructor_ran = true;
    }
}

{
    let mut some_struct = SomeStruct {
        destructor_ran: false,
    };
    assert!(!some_struct.destructor_ran);
    unsafe {
        drop_in_place(&mut some_struct);
    }
    assert!(some_struct.destructor_ran);
}

@eefriedman
Copy link
Contributor

eefriedman commented Jul 17, 2016

Will this example run differently once the transition to MIR happens? If so, in what ways?

Currently, structs which implement Drop have a hidden flag which indicates whether drop() has been called yet. We're planning to get rid of that flag after the transition to MIR. The practical effect is that for your example, drop() would run twice, instead of once like it currently does. See also #34398 . [wrong; see below]

Because of that, we should avoid any example which involves a variable allocated on the stack.

@frewsxcv
Copy link
Member Author

The practical effect is that for your example, drop() would run twice, instead of once like it currently does.

From what I can tell, Drop for SomeStruct gets called twice in both of my examples.

@eefriedman
Copy link
Contributor

eefriedman commented Jul 17, 2016

Oh, right, drop_in_place doesn't do drop filling, sorry, I got confused.

It still makes for a very confusing example that we're dropping the same value twice.

@frewsxcv
Copy link
Member Author

After giving it some thought, and learning more about drop_in_place, I do think my example is not very helpful. I'll keep this open for a day to see if anyone has a better example, otherwise I'll close this.

@frewsxcv frewsxcv closed this Jul 18, 2016
@frewsxcv frewsxcv deleted the drop-in-place branch July 18, 2016 14:16
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

Successfully merging this pull request may close these issues.

2 participants