Skip to content

Commit d107146

Browse files
JarchoJarcho
committed
Reborrow mutable references in explicit_iter_loop 
1 parent f916c29 commit d107146

24 files changed

+159
-58
lines changed

Diff for: ‎clippy_lints/src/attrs.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -808,7 +808,7 @@ fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &Msr
808808
}
809809

810810
fn check_nested_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) {
811-
for item in items.iter() {
811+
for item in items {
812812
if let NestedMetaItem::MetaItem(meta) = item {
813813
if !meta.has_name(sym::any) && !meta.has_name(sym::all) {
814814
continue;
@@ -842,7 +842,7 @@ fn check_nested_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) {
842842
}
843843

844844
fn check_nested_misused_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) {
845-
for item in items.iter() {
845+
for item in items {
846846
if let NestedMetaItem::MetaItem(meta) = item {
847847
if meta.has_name(sym!(features)) && let Some(val) = meta.value_str() {
848848
span_lint_and_sugg(

Diff for: ‎clippy_lints/src/default_numeric_fallback.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> {
161161
let fields_def = &variant.fields;
162162

163163
// Push field type then visit each field expr.
164-
for field in fields.iter() {
164+
for field in *fields {
165165
let bound =
166166
fields_def
167167
.iter()

Diff for: ‎clippy_lints/src/lifetimes.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,7 @@ fn has_where_lifetimes<'tcx>(cx: &LateContext<'tcx>, generics: &'tcx Generics<'_
562562
// if the bounds define new lifetimes, they are fine to occur
563563
let allowed_lts = allowed_lts_from(pred.bound_generic_params);
564564
// now walk the bounds
565-
for bound in pred.bounds.iter() {
565+
for bound in pred.bounds {
566566
walk_param_bound(&mut visitor, bound);
567567
}
568568
// and check that all lifetimes are allowed

Diff for: ‎clippy_lints/src/loops/explicit_into_iter_loop.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ pub(super) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>, call_expr: &Expr<
8484
"it is more concise to loop over containers instead of using explicit \
8585
iteration methods",
8686
"to write this more concisely, try",
87-
format!("{}{}", adjust.display(), object.to_string()),
87+
format!("{}{object}", adjust.display()),
8888
applicability,
8989
);
9090
}

Diff for: ‎clippy_lints/src/loops/explicit_iter_loop.rs

+56-21
Original file line numberDiff line numberDiff line change
@@ -33,26 +33,19 @@ pub(super) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>, call_expr: &Expr<
3333

3434
let mut applicability = Applicability::MachineApplicable;
3535
let object = snippet_with_applicability(cx, self_arg.span, "_", &mut applicability);
36-
let prefix = match adjust {
37-
AdjustKind::None => "",
38-
AdjustKind::Borrow => "&",
39-
AdjustKind::BorrowMut => "&mut ",
40-
AdjustKind::Deref => "*",
41-
AdjustKind::Reborrow => "&*",
42-
AdjustKind::ReborrowMut => "&mut *",
43-
};
4436
span_lint_and_sugg(
4537
cx,
4638
EXPLICIT_ITER_LOOP,
4739
call_expr.span,
4840
"it is more concise to loop over references to containers instead of using explicit \
4941
iteration methods",
5042
"to write this more concisely, try",
51-
format!("{prefix}{object}"),
43+
format!("{}{object}", adjust.display()),
5244
applicability,
5345
);
5446
}
5547

48+
#[derive(Clone, Copy)]
5649
enum AdjustKind {
5750
None,
5851
Borrow,
@@ -76,16 +69,35 @@ impl AdjustKind {
7669
}
7770
}
7871

79-
fn reborrow(mutbl: AutoBorrowMutability) -> Self {
72+
fn reborrow(mutbl: Mutability) -> Self {
73+
match mutbl {
74+
Mutability::Not => Self::Reborrow,
75+
Mutability::Mut => Self::ReborrowMut,
76+
}
77+
}
78+
79+
fn auto_reborrow(mutbl: AutoBorrowMutability) -> Self {
8080
match mutbl {
8181
AutoBorrowMutability::Not => Self::Reborrow,
8282
AutoBorrowMutability::Mut { .. } => Self::ReborrowMut,
8383
}
8484
}
85+
86+
fn display(self) -> &'static str {
87+
match self {
88+
Self::None => "",
89+
Self::Borrow => "&",
90+
Self::BorrowMut => "&mut ",
91+
Self::Deref => "*",
92+
Self::Reborrow => "&*",
93+
Self::ReborrowMut => "&mut *",
94+
}
95+
}
8596
}
8697

8798
/// Checks if an `iter` or `iter_mut` call returns `IntoIterator::IntoIter`. Returns how the
8899
/// argument needs to be adjusted.
100+
#[expect(clippy::too_many_lines)]
89101
fn is_ref_iterable<'tcx>(
90102
cx: &LateContext<'tcx>,
91103
self_arg: &Expr<'_>,
@@ -108,27 +120,50 @@ fn is_ref_iterable<'tcx>(
108120
let self_is_copy = is_copy(cx, self_ty);
109121

110122
if adjustments.is_empty() && self_is_copy {
123+
// Exact type match, already checked earlier
111124
return Some((AdjustKind::None, self_ty));
112125
}
113126

114-
let res_ty = cx.tcx.erase_regions(EarlyBinder::bind(req_res_ty).subst(cx.tcx, typeck.node_substs(call_expr.hir_id)));
115-
if !adjustments.is_empty() && self_is_copy {
116-
if implements_trait(cx, self_ty, trait_id, &[])
117-
&& let Some(ty) = make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [self_ty])
118-
&& ty == res_ty
119-
{
120-
return Some((AdjustKind::None, self_ty));
121-
}
122-
}
123-
127+
let res_ty = cx.tcx.erase_regions(EarlyBinder::bind(req_res_ty)
128+
.subst(cx.tcx, typeck.node_substs(call_expr.hir_id)));
124129
let mutbl = if let ty::Ref(_, _, mutbl) = *req_self_ty.kind() {
125130
Some(mutbl)
126131
} else {
127132
None
128133
};
134+
135+
if !adjustments.is_empty() {
136+
if self_is_copy {
137+
// Using by value won't consume anything
138+
if implements_trait(cx, self_ty, trait_id, &[])
139+
&& let Some(ty) =
140+
make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [self_ty])
141+
&& ty == res_ty
142+
{
143+
return Some((AdjustKind::None, self_ty));
144+
}
145+
} else if let ty::Ref(region, ty, Mutability::Mut) = *self_ty.kind()
146+
&& let Some(mutbl) = mutbl
147+
{
148+
// Attempt to reborrow the mutable reference
149+
let self_ty = if mutbl.is_mut() {
150+
self_ty
151+
} else {
152+
cx.tcx.mk_ref(region, TypeAndMut { ty, mutbl })
153+
};
154+
if implements_trait(cx, self_ty, trait_id, &[])
155+
&& let Some(ty) =
156+
make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [self_ty])
157+
&& ty == res_ty
158+
{
159+
return Some((AdjustKind::reborrow(mutbl), self_ty));
160+
}
161+
}
162+
}
129163
if let Some(mutbl) = mutbl
130164
&& !self_ty.is_ref()
131165
{
166+
// Attempt to borrow
132167
let self_ty = cx.tcx.mk_ref(cx.tcx.lifetimes.re_erased, TypeAndMut {
133168
ty: self_ty,
134169
mutbl,
@@ -157,7 +192,7 @@ fn is_ref_iterable<'tcx>(
157192
make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [target])
158193
&& ty == res_ty
159194
{
160-
Some((AdjustKind::reborrow(mutbl), target))
195+
Some((AdjustKind::auto_reborrow(mutbl), target))
161196
} else {
162197
None
163198
}

Diff for: ‎clippy_lints/src/loops/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,8 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
687687
manual_while_let_some::check(cx, condition, body, span);
688688
}
689689
}
690+
691+
extract_msrv_attr!(LateContext);
690692
}
691693

692694
impl Loops {

Diff for: ‎clippy_lints/src/loops/same_item_push.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ impl<'a, 'tcx> Visitor<'tcx> for SameItemPushVisitor<'a, 'tcx> {
148148
}
149149

150150
fn visit_block(&mut self, b: &'tcx Block<'_>) {
151-
for stmt in b.stmts.iter() {
151+
for stmt in b.stmts {
152152
self.visit_stmt(stmt);
153153
}
154154
}

Diff for: ‎clippy_lints/src/macro_use.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ impl<'tcx> LateLintPass<'tcx> for MacroUseImports {
101101
});
102102
if !id.is_local();
103103
then {
104-
for kid in cx.tcx.module_children(id).iter() {
104+
for kid in cx.tcx.module_children(id) {
105105
if let Res::Def(DefKind::Macro(_mac_type), mac_id) = kid.res {
106106
let span = mac_attr.span;
107107
let def_path = cx.tcx.def_path_str(mac_id);

Diff for: ‎clippy_lints/src/matches/match_wild_err_arm.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm<'
2525
let mut ident_bind_name = kw::Underscore;
2626
if !matching_wild {
2727
// Looking for unused bindings (i.e.: `_e`)
28-
for pat in inner.iter() {
28+
for pat in inner {
2929
if let PatKind::Binding(_, id, ident, None) = pat.kind {
3030
if ident.as_str().starts_with('_') && !is_local_used(cx, arm.body, id) {
3131
ident_bind_name = ident.name;

Diff for: ‎clippy_lints/src/matches/significant_drop_in_scrutinee.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ impl<'a, 'tcx> SigDropChecker<'a, 'tcx> {
140140
}
141141
}
142142

143-
for generic_arg in b.iter() {
143+
for generic_arg in *b {
144144
if let GenericArgKind::Type(ty) = generic_arg.unpack() {
145145
if self.has_sig_drop_attr(cx, ty) {
146146
return true;

Diff for: ‎clippy_lints/src/matches/try_err.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, scrutine
8181
/// Finds function return type by examining return expressions in match arms.
8282
fn find_return_type<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx ExprKind<'_>) -> Option<Ty<'tcx>> {
8383
if let ExprKind::Match(_, arms, MatchSource::TryDesugar) = expr {
84-
for arm in arms.iter() {
84+
for arm in *arms {
8585
if let ExprKind::Ret(Some(ret)) = arm.body.kind {
8686
return Some(cx.typeck_results().expr_ty(ret));
8787
}

Diff for: ‎clippy_lints/src/misc_early/mixed_case_hex_literals.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ pub(super) fn check(cx: &EarlyContext<'_>, lit_span: Span, suffix: &str, lit_sni
1313
return;
1414
}
1515
let mut seen = (false, false);
16-
for ch in lit_snip.as_bytes()[2..=maybe_last_sep_idx].iter() {
16+
for ch in &lit_snip.as_bytes()[2..=maybe_last_sep_idx] {
1717
match ch {
1818
b'a'..=b'f' => seen.0 = true,
1919
b'A'..=b'F' => seen.1 = true,

Diff for: ‎clippy_lints/src/mismatching_type_param_order.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ impl<'tcx> LateLintPass<'tcx> for TypeParamMismatch {
5959
then {
6060
// get the name and span of the generic parameters in the Impl
6161
let mut impl_params = Vec::new();
62-
for p in generic_args.args.iter() {
62+
for p in generic_args.args {
6363
match p {
6464
GenericArg::Type(Ty {kind: TyKind::Path(QPath::Resolved(_, path)), ..}) =>
6565
impl_params.push((path.segments[0].ident.to_string(), path.span)),

Diff for: ‎clippy_lints/src/returns.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ fn check_final_expr<'tcx>(
292292
// (except for unit type functions) so we don't match it
293293
ExprKind::Match(_, arms, MatchSource::Normal) => {
294294
let match_ty = cx.typeck_results().expr_ty(peeled_drop_expr);
295-
for arm in arms.iter() {
295+
for arm in *arms {
296296
check_final_expr(cx, arm.body, semi_spans.clone(), RetReplacement::Unit, Some(match_ty));
297297
}
298298
},

Diff for: ‎clippy_lints/src/significant_drop_tightening.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ impl<'cx, 'sdt, 'tcx> SigDropChecker<'cx, 'sdt, 'tcx> {
333333
return true;
334334
}
335335
}
336-
for generic_arg in b.iter() {
336+
for generic_arg in *b {
337337
if let GenericArgKind::Type(ty) = generic_arg.unpack() {
338338
if self.has_sig_drop_attr(ty) {
339339
return true;

Diff for: ‎clippy_lints/src/trait_bounds.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds {
139139
) = cx.tcx.hir().get_if_local(*def_id);
140140
then {
141141
if self_bounds_map.is_empty() {
142-
for bound in self_bounds.iter() {
142+
for bound in *self_bounds {
143143
let Some((self_res, self_segments, _)) = get_trait_info_from_bound(bound) else { continue };
144144
self_bounds_map.insert(self_res, self_segments);
145145
}
@@ -184,7 +184,7 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds {
184184

185185
// Iterate the bounds and add them to our seen hash
186186
// If we haven't yet seen it, add it to the fixed traits
187-
for bound in bounds.iter() {
187+
for bound in bounds {
188188
let Some(def_id) = bound.trait_ref.trait_def_id() else { continue; };
189189

190190
let new_trait = seen_def_ids.insert(def_id);

Diff for: ‎clippy_lints/src/utils/internal_lints/interning_defined_symbol.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ impl<'tcx> LateLintPass<'tcx> for InterningDefinedSymbol {
7575

7676
for &module in &[&paths::KW_MODULE, &paths::SYM_MODULE] {
7777
for def_id in def_path_def_ids(cx, module) {
78-
for item in cx.tcx.module_children(def_id).iter() {
78+
for item in cx.tcx.module_children(def_id) {
7979
if_chain! {
8080
if let Res::Def(DefKind::Const, item_def_id) = item.res;
8181
let ty = cx.tcx.type_of(item_def_id).subst_identity();

Diff for: ‎clippy_utils/src/qualify_min_const_fn.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ pub fn is_min_const_fn<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, msrv: &Msrv)
6161
body.local_decls.iter().next().unwrap().source_info.span,
6262
)?;
6363

64-
for bb in body.basic_blocks.iter() {
64+
for bb in &*body.basic_blocks {
6565
check_terminator(tcx, body, bb.terminator(), msrv)?;
6666
for stmt in &bb.statements {
6767
check_statement(tcx, body, def_id, stmt)?;
@@ -89,7 +89,7 @@ fn check_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, span: Span) -> McfResult {
8989
return Err((span, "function pointers in const fn are unstable".into()));
9090
},
9191
ty::Dynamic(preds, _, _) => {
92-
for pred in preds.iter() {
92+
for pred in *preds {
9393
match pred.skip_binder() {
9494
ty::ExistentialPredicate::AutoTrait(_) | ty::ExistentialPredicate::Projection(_) => {
9595
return Err((

Diff for: ‎clippy_utils/src/ty.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
277277
false
278278
},
279279
ty::Dynamic(binder, _, _) => {
280-
for predicate in binder.iter() {
280+
for predicate in *binder {
281281
if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate.skip_binder() {
282282
if cx.tcx.has_attr(trait_ref.def_id, sym::must_use) {
283283
return true;

Diff for: ‎clippy_utils/src/usage.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ pub struct ParamBindingIdCollector {
8282
impl<'tcx> ParamBindingIdCollector {
8383
fn collect_binding_hir_ids(body: &'tcx hir::Body<'tcx>) -> Vec<hir::HirId> {
8484
let mut hir_ids: Vec<hir::HirId> = Vec::new();
85-
for param in body.params.iter() {
85+
for param in body.params {
8686
let mut finder = ParamBindingIdCollector {
8787
binding_hir_ids: Vec::new(),
8888
};

Diff for: ‎lintcheck/src/main.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ fn read_crates(toml_path: &Path) -> (Vec<CrateSource>, RecursiveOptions) {
474474
});
475475
} else if let Some(ref versions) = tk.versions {
476476
// if we have multiple versions, save each one
477-
for ver in versions.iter() {
477+
for ver in versions {
478478
crate_sources.push(CrateSource::CratesIo {
479479
name: tk.name.clone(),
480480
version: ver.to_string(),

Diff for: ‎tests/ui/explicit_iter_loop.fixed

+14
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@ fn main() {
1717
for _ in &vec {}
1818
for _ in &mut vec {}
1919

20+
let rvec = &vec;
21+
for _ in rvec {}
22+
23+
let rmvec = &mut vec;
24+
for _ in &*rmvec {}
25+
for _ in &mut *rmvec {}
26+
2027
for _ in &vec {} // these are fine
2128
for _ in &mut vec {} // these are fine
2229

@@ -29,9 +36,13 @@ fn main() {
2936

3037
let ll: LinkedList<()> = LinkedList::new();
3138
for _ in &ll {}
39+
let rll = &ll;
40+
for _ in rll {}
3241

3342
let vd: VecDeque<()> = VecDeque::new();
3443
for _ in &vd {}
44+
let rvd = &vd;
45+
for _ in rvd {}
3546

3647
let bh: BinaryHeap<()> = BinaryHeap::new();
3748
for _ in &bh {}
@@ -137,4 +148,7 @@ fn main() {
137148
let mut x = CustomType;
138149
for _ in &x {}
139150
for _ in &mut x {}
151+
152+
let r = &x;
153+
for _ in r {}
140154
}

0 commit comments

Comments
 (0)