Fix while_let_on_iterator

Reborrow mutable references rather then take a reference to them.
This commit is contained in:
Jason Newcomb 2021-07-30 22:59:20 -04:00
parent f6a5889ffa
commit fe75faa6ee
No known key found for this signature in database
GPG key ID: DA59E8643A37ED06
4 changed files with 71 additions and 4 deletions

View file

@ -7,7 +7,7 @@ use clippy_utils::{
use if_chain::if_chain; use if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor}; use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor};
use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, MatchSource, Node, PatKind, QPath, UnOp}; use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, MatchSource, Mutability, Node, PatKind, QPath, UnOp};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_span::{symbol::sym, Span, Symbol}; use rustc_span::{symbol::sym, Span, Symbol};
@ -48,7 +48,12 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
// borrowed mutably. TODO: If the struct can be partially moved from and the struct isn't used // borrowed mutably. TODO: If the struct can be partially moved from and the struct isn't used
// afterwards a mutable borrow of a field isn't necessary. // afterwards a mutable borrow of a field isn't necessary.
let ref_mut = if !iter_expr.fields.is_empty() || needs_mutable_borrow(cx, &iter_expr, loop_expr) { let ref_mut = if !iter_expr.fields.is_empty() || needs_mutable_borrow(cx, &iter_expr, loop_expr) {
"&mut " if cx.typeck_results().node_type(iter_expr.hir_id).ref_mutability() == Some(Mutability::Mut) {
// Reborrow for mutable references. It may not be possible to get a mutable reference here.
"&mut *"
} else {
"&mut "
}
} else { } else {
"" ""
}; };
@ -69,6 +74,8 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
struct IterExpr { struct IterExpr {
/// The span of the whole expression, not just the path and fields stored here. /// The span of the whole expression, not just the path and fields stored here.
span: Span, span: Span,
/// The HIR id of the whole expression, not just the path and fields stored here.
hir_id: HirId,
/// The fields used, in order of child to parent. /// The fields used, in order of child to parent.
fields: Vec<Symbol>, fields: Vec<Symbol>,
/// The path being used. /// The path being used.
@ -78,12 +85,14 @@ struct IterExpr {
/// the expression might have side effects. /// the expression might have side effects.
fn try_parse_iter_expr(cx: &LateContext<'_>, mut e: &Expr<'_>) -> Option<IterExpr> { fn try_parse_iter_expr(cx: &LateContext<'_>, mut e: &Expr<'_>) -> Option<IterExpr> {
let span = e.span; let span = e.span;
let hir_id = e.hir_id;
let mut fields = Vec::new(); let mut fields = Vec::new();
loop { loop {
match e.kind { match e.kind {
ExprKind::Path(ref path) => { ExprKind::Path(ref path) => {
break Some(IterExpr { break Some(IterExpr {
span, span,
hir_id,
fields, fields,
path: cx.qpath_res(path, e.hir_id), path: cx.qpath_res(path, e.hir_id),
}); });

View file

@ -334,6 +334,29 @@ fn issue7249() {
x(); x();
} }
fn issue7510() {
let mut it = 0..10;
let it = &mut it;
// Needs to reborrow `it` as the binding isn't mutable
for x in &mut *it {
if x % 2 == 0 {
break;
}
}
println!("{}", it.next().unwrap());
struct S<T>(T);
let mut it = 0..10;
let it = S(&mut it);
// Needs to reborrow `it.0` as the binding isn't mutable
for x in &mut *it.0 {
if x % 2 == 0 {
break;
}
}
println!("{}", it.0.next().unwrap());
}
fn main() { fn main() {
let mut it = 0..20; let mut it = 0..20;
for _ in it { for _ in it {

View file

@ -334,6 +334,29 @@ fn issue7249() {
x(); x();
} }
fn issue7510() {
let mut it = 0..10;
let it = &mut it;
// Needs to reborrow `it` as the binding isn't mutable
while let Some(x) = it.next() {
if x % 2 == 0 {
break;
}
}
println!("{}", it.next().unwrap());
struct S<T>(T);
let mut it = 0..10;
let it = S(&mut it);
// Needs to reborrow `it.0` as the binding isn't mutable
while let Some(x) = it.0.next() {
if x % 2 == 0 {
break;
}
}
println!("{}", it.0.next().unwrap());
}
fn main() { fn main() {
let mut it = 0..20; let mut it = 0..20;
while let Some(..) = it.next() { while let Some(..) = it.next() {

View file

@ -111,10 +111,22 @@ LL | while let Some(x) = it.next() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut it` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut it`
error: this loop could be written as a `for` loop error: this loop could be written as a `for` loop
--> $DIR/while_let_on_iterator.rs:339:5 --> $DIR/while_let_on_iterator.rs:341:5
|
LL | while let Some(x) = it.next() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut *it`
error: this loop could be written as a `for` loop
--> $DIR/while_let_on_iterator.rs:352:5
|
LL | while let Some(x) = it.0.next() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut *it.0`
error: this loop could be written as a `for` loop
--> $DIR/while_let_on_iterator.rs:362:5
| |
LL | while let Some(..) = it.next() { LL | while let Some(..) = it.next() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in it` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in it`
error: aborting due to 19 previous errors error: aborting due to 21 previous errors