Skip to content

Commit 7c34ec8

Browse files
committed
Auto merge of #10896 - y21:eager-or-lazy-autoderef, r=blyxyas,xFrednet
consider autoderef through user-defined `Deref` in `eager_or_lazy` Fixes #10462 This PR handles autoderef in the `eager_or_lazy` util module and stops suggesting to change lazy to eager if autoderef in an expression goes through user defined `Deref` impls, e.g. ```rs struct S; impl Deref for S { type Target = (); fn deref(&self) -> &Self::Target { &() } } let _ = Some(()).as_ref().unwrap_or_else(|| &S); // autoderef `&S` -> `&()` ``` changelog: [`unnecessary_lazy_evaluations`]: don't suggest changing lazy evaluation to eager if autoderef goes through user-defined `Deref` r? `@xFrednet` (because of the earlier review in #10864, might help for context here)
2 parents f729147 + e70dd55 commit 7c34ec8

File tree

4 files changed

+81
-36
lines changed

4 files changed

+81
-36
lines changed

clippy_utils/src/eager_or_lazy.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use rustc_hir::def::{DefKind, Res};
1515
use rustc_hir::intravisit::{walk_expr, Visitor};
1616
use rustc_hir::{def_id::DefId, Block, Expr, ExprKind, QPath, UnOp};
1717
use rustc_lint::LateContext;
18+
use rustc_middle::ty::adjustment::Adjust;
1819
use rustc_middle::ty::{self, PredicateKind};
1920
use rustc_span::{sym, Symbol};
2021
use std::cmp;
@@ -114,6 +115,20 @@ fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessS
114115
if self.eagerness == ForceNoChange {
115116
return;
116117
}
118+
119+
// Autoderef through a user-defined `Deref` impl can have side-effects,
120+
// so don't suggest changing it.
121+
if self
122+
.cx
123+
.typeck_results()
124+
.expr_adjustments(e)
125+
.iter()
126+
.any(|adj| matches!(adj.kind, Adjust::Deref(Some(_))))
127+
{
128+
self.eagerness |= NoChange;
129+
return;
130+
}
131+
117132
match e.kind {
118133
ExprKind::Call(
119134
&Expr {

tests/ui/unnecessary_lazy_eval.fixed

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#![allow(clippy::redundant_closure)]
55
#![allow(clippy::bind_instead_of_map)]
66
#![allow(clippy::map_identity)]
7+
#![allow(clippy::needless_borrow)]
78

89
use std::ops::Deref;
910

@@ -81,6 +82,9 @@ fn main() {
8182
let _ = Some(1).unwrap_or(*r);
8283
let b = Box::new(1);
8384
let _ = Some(1).unwrap_or(*b);
85+
// Should lint - Builtin deref through autoderef
86+
let _ = Some(1).as_ref().unwrap_or(&r);
87+
let _ = Some(1).as_ref().unwrap_or(&b);
8488

8589
// Cases when unwrap is not called on a simple variable
8690
let _ = Some(10).unwrap_or(2);
@@ -113,6 +117,9 @@ fn main() {
113117
let _ = Some(1).unwrap_or_else(|| *Issue10437); // Issue10437 has a deref impl
114118
let _ = Some(1).unwrap_or(*Issue10437);
115119

120+
let _ = Some(1).as_ref().unwrap_or_else(|| &Issue10437);
121+
let _ = Some(1).as_ref().unwrap_or(&Issue10437);
122+
116123
// Should not lint - bool
117124
let _ = (0 == 1).then(|| Issue9427(0)); // Issue9427 has a significant drop
118125
let _ = false.then(|| Issue9427FollowUp); // Issue9427FollowUp has a significant drop

tests/ui/unnecessary_lazy_eval.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#![allow(clippy::redundant_closure)]
55
#![allow(clippy::bind_instead_of_map)]
66
#![allow(clippy::map_identity)]
7+
#![allow(clippy::needless_borrow)]
78

89
use std::ops::Deref;
910

@@ -81,6 +82,9 @@ fn main() {
8182
let _ = Some(1).unwrap_or_else(|| *r);
8283
let b = Box::new(1);
8384
let _ = Some(1).unwrap_or_else(|| *b);
85+
// Should lint - Builtin deref through autoderef
86+
let _ = Some(1).as_ref().unwrap_or_else(|| &r);
87+
let _ = Some(1).as_ref().unwrap_or_else(|| &b);
8488

8589
// Cases when unwrap is not called on a simple variable
8690
let _ = Some(10).unwrap_or_else(|| 2);
@@ -113,6 +117,9 @@ fn main() {
113117
let _ = Some(1).unwrap_or_else(|| *Issue10437); // Issue10437 has a deref impl
114118
let _ = Some(1).unwrap_or(*Issue10437);
115119

120+
let _ = Some(1).as_ref().unwrap_or_else(|| &Issue10437);
121+
let _ = Some(1).as_ref().unwrap_or(&Issue10437);
122+
116123
// Should not lint - bool
117124
let _ = (0 == 1).then(|| Issue9427(0)); // Issue9427 has a significant drop
118125
let _ = false.then(|| Issue9427FollowUp); // Issue9427FollowUp has a significant drop

0 commit comments

Comments
 (0)