[map_unwrap_or]: don't lint when referenced variable is moved

[y21]

Fixes #10579.

The previous way of checking if changing map(f).unwrap_or(a) to map_or(a, f) is safe had a flaw when the argument to unwrap_or moves a binding and the map closure references that binding in some way.

It used to simply check if any of the identifiers in the unwrap_or argument are referenced in the map closure, but it didn't consider the case where the moved binding is referred to through references, for example:

let x = vec![1, 2];
let x_ref = &x;
Some(()).map(|_| x_ref.clone()).unwrap_or(x);

This compiles as is, but we cannot change it to map_or. This lint however did suggest changing it, because the simple way of checking if x is referenced anywhere in the map closure fails here. The safest thing to do here imo (what this PR does) is check if the moved value x is referenced anywhere in the body (before the unwrap_or call). One can always create a reference to the value and smuggle them into the closure, without actually referring to x. The original, linked issue shows another one such example:

    let x = vec![1,2,3,0];
    let y = x.strip_suffix(&[0]).map(|s| s.to_vec()).unwrap_or(x);

x.strip_suffix(&[0]) creates a reference to x that is available through s inside of the map closure, so we can't change it to map_or.

changelog: [map_unwrap_or]: don't lint when referenced variable is moved

[rustbot]

r? @xFrednet

(rustbot has picked a reviewer for you, use r? to override)

[xFrednet]

Hey @blyxyas, this looks like another good PR if you want to review it. If you're busy rn, I'm also happy to do the entire review :)

[blyxyas]

If you're busy rn

You understimate me, I may be just out of the hospital, but gimme that bad boi! Clippy 👏 comes 👏 first
(Don't worry, I'm good)

[xFrednet]

Clippy 👏 comes 👏 first

Just making sure, I basically no-lifed Clippy at the start, and only later learned that you need a break every once in a while. That being said, I've no-lifing another project rn again, so it's all back to normal xD

[blyxyas]

@xFrednet I'm sorry but I don't think I can do the full review.
I actually went to the hospital for overdosing on anxiety meds, I've been out for like 4 hours and seeing code rn feels like reading heroglyphics.

If this is urgent, could you review it? If not, I think we'll have to wait a day or two until I can come back to seeing letters and colors in my screen.

[xFrednet]

@blyxyas No worries at all, rest 👏 comes 👏 first! Generally speaking, it's totally fine, to let PRs wait on a review for a bit. I usually try to get to every one within 5 days, but even a week is not a problem. If you know that it'll take time, it's always good to let the author know, but there are no real time-critical issues in Clippy. The changes would need to wait until the next sync anyways.

I've already reviewed some PRs today, and will look at this over the next few days. If you want to pick up the review, when you feel better, you're very welcome. I appreciate you being honest! You can delete that part from your message if you feel it's too personal. Anxiety can suck so much..., if you have any questions, need reassurance or anything, you can always reach out on Zulip as well!

---

@y21 Sorry, for hijacking the PR comments for this discussion. You'll get a review in the next couple of days :D

[y21]

Sorry, for hijacking the PR comments for this discussion. You'll get a review in the next couple of days :D

it's all good, i won't be home this weekend anyways, so take your time :)

[blyxyas]

LGTM! Thanks, pretty hard issue to solve ❤️

cc @xFrednet

[xFrednet]

Looks good to me! Thank you for the PR :D

And the review @blyxyas

@bors r=blyxyas,xFrednet

[bors]

📌 Commit 9d58a42 has been approved by blyxyas,xFrednet

It is now in the queue for this repository.

[bors]

⌛ Testing commit 9d58a42 with merge a59236f...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: blyxyas,xFrednet
Pushing a59236f to master...