fix 5707

[lengyijun]

changelog: [`redundant_clone`], fix #5707

Root problem of #5707 :

&2:&mut HashMap = &mut _4;
&3:&str = & _5;
_1 = HashMap::insert(move _2,move _3, _);

generate PossibleBorrower(_2,_1) and PossibleBorrower(_3,_1).

However, it misses PossibleBorrower(_3,_2).

My solution to #5707 :

When meet a function call, we should:

1. build PossibleBorrower between borrow parameters and return value (currently)
2. build PossibleBorrower between immutable borrow parameters and mutable borrow parameters (add)
3. build PossibleBorrower inside mutable borrow parameters (add)

For example:

_2: &mut _22;
_3: &mut _;
_4: & _;
_5: & _;
_1 = call(move _2, move _3, move _4, move _5);

we need to build

1. return value with parameter(current implementataion)
   PossibleBorrower(_2,_1)
   PossibleBorrower(_3,_1)
   PossibleBorrower(_4,_1)
   PossibleBorrower(_5,_1)

2. between mutable borrow and immutable borrow
   PossibleBorrower(_4,_2)
   PossibleBorrower(_5,_2)
   PossibleBorrower(_4,_3)
   PossibleBorrower(_5,_3)

3. between mutable borrow and mutable borrow
   PossibleBorrower(_3,_2)
   PossibleBorrower(_2,_3)

But that's not enough.
Modification to _2 actually apply to _22.
So I write a PossibleBorrowed visitor, which tracks (borrower => possible borrowed) relation.
For example (_2 => _22).
However, a lot of problems exist here.

Known Problems:

1. not sure all &mut's origin are collected.
   I'm not sure how to deal with &mut when meet a function call, so I didn't do it currently.
   Also, my implement is not flow sensitive, so it's not accurate.

foo(_2:&mut _, _3: &_)

This pr doesn't count _3 as origin of _2.

2. introduce false negative
   foo(_2, _3) will emit PossibleBorrower(_3,_2) in this pr, but _3 and _2 may not have relation.
   Clippy may feel that _3 is still in use because of _2, but actually, _3 is on longer needed and can be moved.

Insight

The key problem is determine where every &mut come from accurately.
I think Polonius is an elegant solution to it. Polonius is flow sensitive and accurate.
But I'm uncertain about whether we can import Polonius in rust-clippy currently.
This pr actually is part of Polonius' functionality, I think.

TODO

1. cargo test can't pass yet due to similar variable name

[rust-highfive]

r? @giraffate

(rust-highfive has picked a reviewer for you, use r? to override)

[0969397110]

H

[flip1995]

There are dogfood errors and you have to run cargo dev fmt.

Since this change heavily involves knowledge of MIR, I'd like to give this review to r? @oli-obk. Also cc @sinkuu since you've done the most work on this lint until now.

[oli-obk]

I won't get to this for the next three weeks

[oli-obk]

Will introduce false negative

can you add a test showing this?

not sure all &mut's origin are collected.

you already mentioned feeding &mut into function calls and getting something out. A similar situation occurs with deref projections. Dereferencing a raw pointer can subvert any checks we could possibly make.

The implementation of this PR lgtm as far as in "is an improvement to the status quo", as you're rejecting more code and now allowing anything that wasn't allowed before. I will merge it, but I think we (wg-mir-opt) should figure out how to make writing and analyzing mir optimizations better.

One of my main worries with this lint (and other mir analyses) is that it is hard to reason about their behaviour at the source level. We generate and track a lot of information, but we cannot "visualize" (textually or otherwise) this information and thus analyze it in detail. While there are such visualizations for specific analyses, we should figure out something generalizeable.

[oli-obk]

Clippy dogfood is failing, and likely it would be good to rebase this PR first. After that r=me

[lengyijun]

Thanks for reviewing such a longthy pr!

can you add a test showing this?

Newly added test: false_negative_5707

I think we (wg-mir-opt) should figure out how to make writing and analyzing mir optimizations better.

I'd like to help it. You can appoint specific job for me. I can make a try.

[oli-obk]

You still have the dogfood test failure (running clippy on clippy) to address before we can merge this.

I'd like to help it. You can appoint specific job for me. I can make a try.

Let's talk about this in https://rust-lang.zulipchat.com/#narrow/stream/189540-t-compiler.2Fwg-mir-opt/topic/debugging.20tools.20for.20dataflow.20and.20mir.20opts

[oli-obk]

@bors r+

Thanks!

[bors]

📌 Commit 1091002 has been approved by oli-obk

[bors]

⌛ Testing commit 1091002 with merge f07feca...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: oli-obk
Pushing f07feca to master...