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

Adding 'clone to satisfy the borrow checker' anti-pattern #110

Merged
merged 15 commits into from Mar 29, 2021
Merged
1 change: 1 addition & 0 deletions SUMMARY.md
Expand Up @@ -36,6 +36,7 @@
- [Visitor](./patterns/visitor.md)

- [Anti-patterns](./anti_patterns/index.md)
- [Clone to satisfy the borrow checker](./anti_patterns/borrow_clone.md)
- [`#[deny(warnings)]`](./anti_patterns/deny-warnings.md)
- [Deref Polymorphism](./anti_patterns/deref.md)

Expand Down
54 changes: 54 additions & 0 deletions anti_patterns/borrow_clone.md
@@ -0,0 +1,54 @@
# Clone to satisfy the borrow checker

## Description

The borrow checker prevents Rust users from developing otherwise unsafe code by
ensuring that either: only one mutable reference exists, or potentially many but
all immutable references exist. If the code written does not hold true to these
conditions, this anti-pattern arises when the developer resolves the compiler
error by cloning the variable.

## Example

```rust
// define any variable
let mut x = 5;

// Borrow `x` -- but clone it first
let y = &mut (x.clone());

// perform some action on the borrow to prevent rust from optimizing this
//out of existence
*y += 1;

// without the x.clone() two lines prior, this line would fail on compile as
// x has been borrowed
// thanks to x.clone(), x was never borrowed, and this line will run.
println!("{}", x);
```

## Motivation

It is tempting, particularly for beginners, to use this pattern to resolve
confusing issues with the borrow checker. However, there are serious
consequences. Using `.clone()` causes a copy of the data to be made. Any changes
between the two are not synchronized -- as if two completely separate variables
exist.

There are special cases -- `Rc<T>` is designed to handle clones intelligently.
simonsan marked this conversation as resolved.
Show resolved Hide resolved
It internally manages exactly one copy of the data, and cloning it will only
clone the reference.

In general, clones should be deliberate, with full understanding of the
consequences. If a clone is used to make a borrow checker error disappear,
that's a good indication this anti-pattern may be in use.

If an unnecessary clone is suspected, The Rust Book's chapter on Ownership
should be understood fully before assessing whether the clone is required or not.

simonsan marked this conversation as resolved.
Show resolved Hide resolved
Also be sure to always run `cargo clippy` in your project, which will detect some cases in which `.clone()` is not necessary, like [1](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone), [2](https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy), [3](https://rust-lang.github.io/rust-clippy/master/index.html#map_clone) or [4](https://rust-lang.github.io/rust-clippy/master/index.html#clone_double_ref).

## See also
simonsan marked this conversation as resolved.
Show resolved Hide resolved

[The Rust Book: Ownership, which describes how the borrow checker behaves](https://doc.rust-lang.org/book/ownership.html)
[`Rc<T>` documentation, which handles .clone() intelligently](http://doc.rust-lang.org/std/rc/)
simonsan marked this conversation as resolved.
Show resolved Hide resolved