From c9c5f700240cea64901e540cdcd87df1171b07ff Mon Sep 17 00:00:00 2001
From: simonsan <14062932+simonsan@users.noreply.github.com>
Date: Mon, 29 Mar 2021 23:51:47 +0200
Subject: [PATCH] Adding 'clone to satisfy the borrow checker' anti-pattern
 (#110)

---
 SUMMARY.md                    |  1 +
 anti_patterns/borrow_clone.md | 73 +++++++++++++++++++++++++++++++++++
 idioms/mem-replace.md         |  6 +--
 3 files changed, 76 insertions(+), 4 deletions(-)
 create mode 100644 anti_patterns/borrow_clone.md

diff --git a/SUMMARY.md b/SUMMARY.md
index 696c5ca..488ec66 100644
--- a/SUMMARY.md
+++ b/SUMMARY.md
@@ -39,6 +39,7 @@
     - [Type Consolidation into Wrappers](./patterns/ffi/wrappers.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)
 
diff --git a/anti_patterns/borrow_clone.md b/anti_patterns/borrow_clone.md
new file mode 100644
index 0000000..4ee9678
--- /dev/null
+++ b/anti_patterns/borrow_clone.md
@@ -0,0 +1,73 @@
+# 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.
+It internally manages exactly one copy of the data, and cloning it will only
+clone the reference.
+
+There is also `Arc<T>` which provides shared ownership of a value of type T
+that is allocated in the heap. Invoking `.clone()` on `Arc` produces a new `Arc`
+instance, which points to the same allocation on the heap as the source `Arc`,
+while increasing a reference count.
+
+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.
+
+Even though `.clone()` is an indication of a bad pattern, sometimes
+**it is fine to write inefficient code**, in cases such as when:
+
+- the developer is still new to ownership
+- the code doesn't have great speed or memory constraints
+  (like hackathon projects or prototypes)
+- satisfying the borrow checker is really complicated and you prefer to
+  optimize readability over performance
+
+If an unnecessary clone is suspected, The [Rust Book's chapter on Ownership](https://doc.rust-lang.org/book/ownership.html)
+should be understood fully before assessing whether the clone is required or not.
+
+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
+
+- [`mem::{take(_), replace(_)}` to keep owned values in changed enums](../idioms/mem-replace.md)
+- [`Rc<T>` documentation, which handles .clone() intelligently](http://doc.rust-lang.org/std/rc/)
+- [`Arc<T>` documentation, a thread-safe reference-counting pointer](https://doc.rust-lang.org/std/sync/struct.Arc.html)
+- [Tricks with ownership in Rust](https://web.archive.org/web/20210120233744/https://xion.io/post/code/rust-borrowchk-tricks.html)
diff --git a/idioms/mem-replace.md b/idioms/mem-replace.md
index 49d6891..3649b6d 100644
--- a/idioms/mem-replace.md
+++ b/idioms/mem-replace.md
@@ -116,7 +116,5 @@ like Indiana Jones, replacing the artifact with a bag of sand.
 
 ## See also
 
-This gets rid of the [Clone to satisfy the borrow checker] antipattern in a
-specific case.
-
-[Clone to satisfy the borrow checker](TODO: Hinges on PR #23)
+This gets rid of the [Clone to satisfy the borrow checker](../anti_patterns/borrow_clone.md)
+antipattern in a specific case.