`make_copy` and `make_deep_copy` require unnecessary _mut_ self

[kelko]

The methods make_copy and make_deep_copy require a mutable reference to self. But looking at the code I don't see an obvious reason for this. But this requirement makes usage of that method more complicated. Is there a reason behind that requirement I just don't see? Or can mut be dropped? (I changed it on a local copy and at least all tests still compiled and ran green)

[RazrFalcon]

This is just a convention. In simple cases the compiler would not allow you to copy borrowed value, which will lead to a panic otherwise. No other reason as far as I remember.

[kelko]

I would argue, that this serves a minor role at best (and might be simply wrong at worst).

With current code you can still write incorrect stuff like:

#[test]
fn incorrect_borrowing() {
    let node1 = Node::new(1);
    let mut node2 = Node::new(2);

    let mut node1_clone1 = Node::clone(&node1);
    let mut node1_clone2 = Node::clone(&node1);
    let _foo = node1.borrow();
    let _bar = node1_clone2.borrow_mut();

    node2.append(node1_clone1.make_deep_copy());

    assert_eq!(
        format!("{:?}", TreePrinter(node2)),
        "2
    1
"
    );
}

And the compiler won't complain although you run into a panic. Because of node1_clone2.borrow_mut(). But once you remove that faulty line the code does not only compile, but actually work (since make_copy also uses borrow not borrow_mut, which may happen multiple times).

Now: If you remove the mut part of the reference for make_copy/make_deep_copy the code above still works 1:1 as before. But then suddenly also the code below passes the compiler (while it won't pass in the &mut self version):

let node1 = Node::new(1);
let mut node2 = Node::new(2);
let _foo = node1.borrow();

node2.append(node1.make_deep_copy());

So the compiler won't allow me compile code, which actually and truly work. To me that concludes that mut rather seems to work against how the code actually can be used. Instead of helping correct usage. Which I find weird.

[RazrFalcon]

Then all methods should not be marked as mut as well. It's not make_copy specific issue, this is just how the API was designed.

I'm fine with this change. You're free to send a patch that removes mut from all methods (were possible).