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

Tweak std::rc docs #36571

Merged
merged 1 commit into from
Sep 22, 2016
Merged

Tweak std::rc docs #36571

merged 1 commit into from
Sep 22, 2016

Conversation

kmcallister
Copy link
Contributor

//!
//! `Rc<T>` automatically dereferences to `T` (via the [`Deref`][deref] trait),
//! so you can call `T`'s methods on a value of type `Rc<T>`. The methods of
//! `Rc` itself, such as `downgrade`, are called using function-like syntax
Copy link
Member

Choose a reason for hiding this comment

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

Not a concrete suggestion, but these are called associated functions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool; added a note.

//! Rc::downgrade(&my_rc);
//! ```
//!
//! to avoid name clashes with `T`'s methods.
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, in order to prevent the code block from breaking up the sentence, it could also be something like:

In order to avoid name clashes with T's methods, the methods of Rc itself, such as downgrade, are called using function-like syntax:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea; fixed.

@@ -570,16 +681,16 @@ impl<T: ?Sized + PartialEq> PartialEq for Rc<T> {
///
/// let five = Rc::new(5);
///
/// five == Rc::new(5);
/// assert!(five == Rc::new(5));
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, assert_eq!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with == because we're talking about the definition of ==. Of course, assert_eq! uses == but it's an extra layer of indirection.

@@ -588,7 +699,7 @@ impl<T: ?Sized + PartialEq> PartialEq for Rc<T> {
///
/// let five = Rc::new(5);
///
/// five != Rc::new(5);
/// assert!(five != Rc::new(6));
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, assert_eq!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think there's an assert_neq!. Anyway, see above.

Copy link
Member

Choose a reason for hiding this comment

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

Just kidding, missed the !=. On a side note: rust-lang/rfcs#1653

@kmcallister
Copy link
Contributor Author

btw, I'm updating Arc docs to match. That will be a separate PR.

@kmcallister
Copy link
Contributor Author

Rebased and squashed.

@GuillaumeGomez
Copy link
Member

Thanks a lot!

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Sep 21, 2016

📌 Commit c316ae5 has been approved by GuillaumeGomez

sophiajt pushed a commit to sophiajt/rust that referenced this pull request Sep 21, 2016
sophiajt pushed a commit to sophiajt/rust that referenced this pull request Sep 22, 2016
bors added a commit that referenced this pull request Sep 22, 2016
Rollup of 7 pull requests

- Successful merges: #36330, #36423, #36539, #36571, #36589, #36600, #36632
- Failed merges:
@bors bors merged commit c316ae5 into rust-lang:master Sep 22, 2016
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Oct 7, 2016
Update Arc docs to match new Rc docs

`Rc` docs were updated in rust-lang#36571. This applies similar changes to `Arc` docs.

r? @GuillaumeGomez
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.

4 participants