-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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 Rc<T> #19112
Document Rc<T> #19112
Conversation
@@ -201,7 +211,17 @@ impl<T> Rc<T> { | |||
} | |||
} | |||
|
|||
/// Downgrades the reference-counted pointer to a weak reference. | |||
/// Downgrades the `Rc<T>` to a `Weak` reference. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You seem to use just Rc
everywhere else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, its just inconsistent. shrug
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we have no guideline here, I don't think :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc @aturon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slight preference for Rc<T>
, but feel free to make your own doc conventions for this kind of thing -- you can batch into a PR against https://github.com/rust-lang/rust-guidelines or an RFC.
Rebased and squashed. |
3bc3cab
to
a396a98
Compare
/// Drops the `Rc<T>`. | ||
/// | ||
/// This will decrement the strong reference count. If the strong reference count becomes zero | ||
/// and the only other references are `Weak<T>` ones, `drop` the inner value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drop
s?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's more consistent.
/// A weak reference to a reference-counted pointer. | ||
/// A weak version of `Rc<T>`. | ||
/// | ||
/// Weak references do not count when determining if the inner value should be deallocated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another tricky case of deallocate vs drop.
@gankro updated, I think! |
@@ -60,8 +58,8 @@ | |||
//! // Despite dropping gadget_owner, we're still able to print out the name of | |||
//! // the Owner of the Gadgets. This is because we've only dropped the | |||
//! // reference count object, not the Owner it wraps. As long as there are | |||
//! // other Rc objects pointing at the same Owner, it will stay alive. Notice | |||
//! // that the Rc wrapper around Gadget.owner gets automatically dereferenced | |||
//! // other `Rc<T>` objects pointing at the same Owner, it will remain allocaed. Notice |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/allocaed/allocated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's what I get for touching other people's prose 😉
updated!
This commit introduces a bunch of documentation, fixes some consistency issues, and just basically brings Rc<T> and Weak<T> up to snuff.
/// | ||
/// Unique ownership means that there are no other `Rc` or `Weak` values | ||
/// that share the same contents. | ||
/// rc::is_unique(&five); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No asserts still feels weird to me. shrug
This basically LGTM now. I don't think we should bother doc'ing le/ge/eq/etc. I also just realized we should just be able to axe those in favour of auto or manual deref. The only downside is
Not sure what the total plans for operators like that is. I'll defer to @japaric and @aturon on that matter. |
I only documneted them because it does something interesting, which seemed like the bar. Not opposed to axing them. |
It does exactly what you'd expect every smart pointer to do, right? |
No description provided.