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

Alter std::cell::Cell::get_mut documentation #86397

Merged
merged 1 commit into from
Jun 19, 2021

Conversation

Eosis
Copy link
Contributor

@Eosis Eosis commented Jun 17, 2021

I felt that there was some inconsistency between between Cell and RefCell with regards to their get_mut method documentation: RefCell flags this method as "unusual" in that it takes &mut self, while Cell does not. I attempted to flag this in Cells documentation as well, and point to RefCells method in the case where it is required.

Find relevant parts of docs and the new version below.

The current docs for Cell::get_mut:

Returns a mutable reference to the underlying data.
This call borrows Cell mutably (at compile-time) which guarantees that we possess the only reference.

And RefCell::get_mut:

Returns a mutable reference to the underlying data.
This call borrows RefCell mutably (at compile-time) so there is no need for dynamic checks.
However be cautious: this method expects self to be mutable, which is generally not the case when using a RefCell. Take a look at the borrow_mut method instead if self isn’t mutable.
Also, please be aware that this method is only for special circumstances and is usually not what you want. In case of doubt, use borrow_mut instead.

My attempt to make Cell::get_mut clearer:

Returns a mutable reference to the underlying data.
This call borrows Cell mutably (at compile-time) which guaranteesthat we possess the only reference.
However be cautious: this method expects self to be mutable, which is generally not the case when using a Cell. If you require interior mutability by reference, consider using RefCell which provides run-time checked mutable borrows through its borrow_mut method.

I find this more consistent with RefCell's equivalent method.
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @scottmcm (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 17, 2021
Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

Makes sense, thanks!

@JohnTitor
Copy link
Member

r? @JohnTitor @bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 18, 2021

📌 Commit 7cadf7b has been approved by JohnTitor

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 18, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 19, 2021
Alter std::cell::Cell::get_mut documentation

I felt that there was some inconsistency between between Cell and RefCell with regards to their `get_mut` method documentation: `RefCell` flags this method as "unusual" in that it takes `&mut self`, while `Cell` does not. I attempted to flag this in `Cell`s documentation as well, and point to `RefCell`s method in the case where it is required.

Find relevant parts of docs and the new version below.

The current docs for `Cell::get_mut`:
> Returns a mutable reference to the underlying data.
This call borrows Cell mutably (at compile-time) which guarantees that we possess the only reference.

And `RefCell::get_mut`:
> Returns a mutable reference to the underlying data.
 This call borrows `RefCell` mutably (at compile-time) so there is no need for dynamic checks.
However be cautious: this method expects self to be mutable, which is generally not the case when using a `RefCell`. Take a look at the `borrow_mut` method instead if self isn’t mutable.
Also, please be aware that this method is only for special circumstances and is usually not what you want. In case of doubt, use `borrow_mut` instead.

My attempt to make `Cell::get_mut` clearer:
> Returns a mutable reference to the underlying data.
This call borrows `Cell` mutably (at compile-time) which guaranteesthat we possess the only reference.
However be cautious: this method expects `self` to be mutable, which is generally not the case when using a `Cell`. If you require interior mutability by reference, consider using `RefCell` which provides run-time checked mutable borrows through its `borrow_mut` method.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 19, 2021
Rollup of 9 pull requests

Successful merges:

 - rust-lang#86136 (Stabilize span_open() and span_close().)
 - rust-lang#86359 (Use as_secs_f64 in JunitFormatter)
 - rust-lang#86370 (Fix rustdoc stabilized versions layout)
 - rust-lang#86397 (Alter std::cell::Cell::get_mut documentation)
 - rust-lang#86407 (Use `map_or` instead of open-coding it)
 - rust-lang#86425 (Update rustversion to 1.0.5)
 - rust-lang#86440 (Update library tracking issue for libs-api rename.)
 - rust-lang#86444 (Fix ICE with `#[repr(simd)]` on enum)
 - rust-lang#86453 (stdlib: Fix typo in internal RefCell docs )

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 90e82c9 into rust-lang:master Jun 19, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jun 19, 2021
@Eosis Eosis deleted the alter-cell-docs branch June 19, 2021 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants