Skip to content

Lint against RefCell::borrow/borrow_mut on &mut RefCell #9044

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

Open
RalfJung opened this issue Jun 24, 2022 · 0 comments
Open

Lint against RefCell::borrow/borrow_mut on &mut RefCell #9044

RalfJung opened this issue Jun 24, 2022 · 0 comments
Labels
A-lint Area: New lints

Comments

@RalfJung
Copy link
Member

What it does

On this code:

use std::cell::RefCell;

pub fn foo(x: &mut RefCell<i32>) {
    *x.borrow_mut() += 1;
}

the lint would suggest to use get_mut() rather than borrow_mut(). (Same for borrow(), of course.)

Lint Name

unnecessary_runtime_borrow

Category

perf

Advantage

  • Removes unnecessary run-time borrow tracking of RefCell

Drawbacks

None that I know if

Example

See above.

Mutex and RwLock also have get_mut functions, so the same kind of lint could be added for them.

@RalfJung RalfJung added the A-lint Area: New lints label Jun 24, 2022
lukaslueg added a commit to lukaslueg/rust-clippy that referenced this issue Sep 3, 2022
lukaslueg added a commit to lukaslueg/rust-clippy that referenced this issue Sep 3, 2022
lukaslueg added a commit to lukaslueg/rust-clippy that referenced this issue Sep 3, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Sep 19, 2022
Tone down explanation on RefCell::get_mut

The language around `RefCell::get_mut` is remarkably sketchy and especially to the novice seems to quite strongly discourage using the method ("be cautious", "Also, please be aware", "special circumstances", "usually not what you want"). It was added six years ago in rust-lang#40634 due to confusion about when to use `get_mut` and `borrow_mut`.

While its signature limits the use-cases for `get_mut`, there is no chance for a safety footgun, and readers can be made aware of `borrow_mut` more softly. I've also just sent a [PR](rust-lang/rust-clippy#9044) to lint situations where `get_mut` could be used to improve ergonomics and performance.

So this PR tones down the language around `get_mut` and also brings it more in line with [`std::sync::Mutex::get_mut()`](https://doc.rust-lang.org/stable/std/sync/struct.Mutex.html#method.get_mut).
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 19, 2022
Tone down explanation on RefCell::get_mut

The language around `RefCell::get_mut` is remarkably sketchy and especially to the novice seems to quite strongly discourage using the method ("be cautious", "Also, please be aware", "special circumstances", "usually not what you want"). It was added six years ago in rust-lang#40634 due to confusion about when to use `get_mut` and `borrow_mut`.

While its signature limits the use-cases for `get_mut`, there is no chance for a safety footgun, and readers can be made aware of `borrow_mut` more softly. I've also just sent a [PR](rust-lang/rust-clippy#9044) to lint situations where `get_mut` could be used to improve ergonomics and performance.

So this PR tones down the language around `get_mut` and also brings it more in line with [`std::sync::Mutex::get_mut()`](https://doc.rust-lang.org/stable/std/sync/struct.Mutex.html#method.get_mut).
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Feb 10, 2023
Tone down explanation on RefCell::get_mut

The language around `RefCell::get_mut` is remarkably sketchy and especially to the novice seems to quite strongly discourage using the method ("be cautious", "Also, please be aware", "special circumstances", "usually not what you want"). It was added six years ago in #40634 due to confusion about when to use `get_mut` and `borrow_mut`.

While its signature limits the use-cases for `get_mut`, there is no chance for a safety footgun, and readers can be made aware of `borrow_mut` more softly. I've also just sent a [PR](rust-lang/rust-clippy#9044) to lint situations where `get_mut` could be used to improve ergonomics and performance.

So this PR tones down the language around `get_mut` and also brings it more in line with [`std::sync::Mutex::get_mut()`](https://doc.rust-lang.org/stable/std/sync/struct.Mutex.html#method.get_mut).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant