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

Document drop more. #42159

Merged
merged 5 commits into from
May 25, 2017
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions src/libcore/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,14 @@ use marker::Unsize;
/// The `Drop` trait is used to run some code when a value goes out of scope.
/// This is sometimes called a 'destructor'.
///
/// When a value goes out of scope, if it implements this trait, it will have
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit repetitive w.r.t "when a value goes out of scope".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine, as the first line is a summary, and this is the full explanation.

/// its `drop` method called. Then any fields the value contains will also
/// be dropped recursively.
///
/// Because of the recursive dropping, even for types that do not implement
/// this trait, you do not need to implement this trait unless your type
/// needs its own destructor logic.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"even for types that do not implement this trait, you do not need to implement this trait"

doesn't parse to me :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what happens when you stare at the screen for half an hour wondering what to write.

///
/// # Examples
///
/// A trivial implementation of `Drop`. The `drop` method is called when `_x`
Expand All @@ -171,6 +179,44 @@ use marker::Unsize;
/// let _x = HasDrop;
/// }
/// ```
///
/// Showing the recursive nature of `Drop`. When `outer` goes out of scope, the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First sentence is incomplete?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only as much as the first sentence to the previous example which says "A trivial implementation of Drop".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah; I think this is fine, though personally, I tend to use fragments like this for the short ones, and since there's more here, I'd probably make it a full sentence. NBD

/// `drop` method will be called for `Outer` and then the `drop` method for
/// `Inner` will be called. Therefore `main` prints `Dropping Outer!` and then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

repetitive wording "the drop method will be called for Outer and then the drop method for Inner will be called."
Why not "the drop method will be called first for Outer, then for Inner"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can do.

/// `Dropping Inner!`.
///
/// ```
/// struct Inner;
/// struct Outer(Inner);
///
/// impl Drop for Inner {
/// fn drop(&mut self) {
/// println!("Dropping Inner!");
/// }
/// }
///
/// impl Drop for Outer {
/// fn drop(&mut self) {
/// println!("Dropping Outer!");
/// }
/// }
///
/// fn main() {
/// let _x = Outer(Inner);
/// }
/// ```
///
/// Because variables are dropped in the reverse order they are declared,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "declared" the right term here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

/// `main` will print `Declared second!` and then `Declared first!`.
///
/// ```
/// struct PrintOnDrop(&'static str);
///
/// fn main() {
/// let _first = PrintOnDrop("Declared first!");
/// let _second = PrintOnDrop("Declared second!");
/// }
/// ```
#[lang = "drop"]
#[stable(feature = "rust1", since = "1.0.0")]
pub trait Drop {
Expand Down