Skip to content

Commit 18f0d44

Browse files
committed
Deny clippy::clone_on_ref_ptr
**This Commit** Denies the use of `.clone()` on reference counted pointers as advised by [this `clippy` lint][0] and [The Rust Book][1]. It also addresses instances where the lint was violated. **Why?** It's best practice to make clear the clone is cheap. There is [some debate][2] on the issue and the [standard library no longer explicitly recommends it][3] but it is still noted as more idiomatic. In any case, for us there are no issues making the change and if there is a possibility for being more expressive, we should take it. See #6 (comment) for more details. [0]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_ref_ptr [1]: https://doc.rust-lang.org/book/ch15-04-rc.html#using-rct-to-share-data [2]: rust-lang/rust-clippy#2048 [3]: rust-lang/rust#76138
1 parent 1cba1fe commit 18f0d44

File tree

2 files changed

+29
-25
lines changed

2 files changed

+29
-25
lines changed

src/functional.rs

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -86,21 +86,21 @@ impl<K, V> Tree<K, V> {
8686
Tree::Node(n) => match key.cmp(&n.key) {
8787
cmp::Ordering::Less => Tree::Node(Node {
8888
left: Rc::new(n.left.insert(key, value)),
89-
key: n.key.clone(),
90-
right: n.right.clone(),
91-
value: n.value.clone(),
89+
key: Rc::clone(&n.key),
90+
right: Rc::clone(&n.right),
91+
value: Rc::clone(&n.value),
9292
}),
9393
cmp::Ordering::Equal => Tree::Node(Node {
9494
value: Rc::new(value),
95-
key: n.key.clone(),
96-
left: n.left.clone(),
97-
right: n.right.clone(),
95+
key: Rc::clone(&n.key),
96+
left: Rc::clone(&n.left),
97+
right: Rc::clone(&n.right),
9898
}),
9999
cmp::Ordering::Greater => Tree::Node(Node {
100100
right: Rc::new(n.right.insert(key, value)),
101-
key: n.key.clone(),
102-
left: n.left.clone(),
103-
value: n.value.clone(),
101+
key: Rc::clone(&n.key),
102+
left: Rc::clone(&n.left),
103+
value: Rc::clone(&n.value),
104104
}),
105105
},
106106
}
@@ -162,9 +162,9 @@ impl<K, V> Tree<K, V> {
162162
Tree::Node(n) => match k.cmp(&n.key) {
163163
cmp::Ordering::Less => Tree::Node(Node {
164164
left: Rc::new(n.left.delete(k)),
165-
key: n.key.clone(),
166-
right: n.right.clone(),
167-
value: n.value.clone(),
165+
key: Rc::clone(&n.key),
166+
right: Rc::clone(&n.right),
167+
value: Rc::clone(&n.value),
168168
}),
169169
cmp::Ordering::Equal => match (n.left.as_ref(), n.right.as_ref()) {
170170
(Tree::Leaf, Tree::Leaf) => Tree::Leaf,
@@ -179,17 +179,17 @@ impl<K, V> Tree<K, V> {
179179
let (pred_key, pred_val, new_left) = left_child.delete_largest();
180180
Tree::Node(Node {
181181
left: new_left,
182-
right: n.right.clone(),
182+
right: Rc::clone(&n.right),
183183
key: pred_key,
184184
value: pred_val,
185185
})
186186
}
187187
},
188188
cmp::Ordering::Greater => Tree::Node(Node {
189189
right: Rc::new(n.right.delete(k)),
190-
key: n.key.clone(),
191-
left: n.left.clone(),
192-
value: n.value.clone(),
190+
key: Rc::clone(&n.key),
191+
left: Rc::clone(&n.left),
192+
value: Rc::clone(&n.value),
193193
}),
194194
},
195195
}
@@ -214,10 +214,10 @@ pub struct Node<K, V> {
214214
impl<K, V> Clone for Node<K, V> {
215215
fn clone(&self) -> Self {
216216
Self {
217-
key: self.key.clone(),
218-
value: self.value.clone(),
219-
left: self.left.clone(),
220-
right: self.right.clone(),
217+
key: Rc::clone(&self.key),
218+
left: Rc::clone(&self.left),
219+
right: Rc::clone(&self.right),
220+
value: Rc::clone(&self.value),
221221
}
222222
}
223223
}
@@ -239,7 +239,11 @@ impl<K, V> Node<K, V> {
239239
K: cmp::Ord,
240240
{
241241
match self.right.as_ref() {
242-
Tree::Leaf => (self.key.clone(), self.value.clone(), self.left.clone()),
242+
Tree::Leaf => (
243+
Rc::clone(&self.key),
244+
Rc::clone(&self.value),
245+
Rc::clone(&self.left),
246+
),
243247
Tree::Node(r) => {
244248
let (key, value, sub) = r.delete_largest();
245249

@@ -248,9 +252,9 @@ impl<K, V> Node<K, V> {
248252
value,
249253
Rc::new(Tree::Node(Node {
250254
right: sub,
251-
left: self.left.clone(),
252-
key: self.key.clone(),
253-
value: self.value.clone(),
255+
key: Rc::clone(&self.key),
256+
left: Rc::clone(&self.left),
257+
value: Rc::clone(&self.value),
254258
})),
255259
)
256260
}

src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,6 @@
2323
//! in the tree. BSTs also naturally support sorted iteration by visiting the
2424
//! left subtree, then the subtree root, then the right subtree.
2525
26-
#![deny(missing_docs)]
26+
#![deny(missing_docs, clippy::clone_on_ref_ptr)]
2727

2828
pub mod functional;

0 commit comments

Comments
 (0)